Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[1.20.2] Initial port work; port resource loader #336

Closed
wants to merge 9 commits into from
Closed

[1.20.2] Initial port work; port resource loader #336

wants to merge 9 commits into from

Conversation

TheGlitch76
Copy link
Contributor

No description provided.

EnnuiL
EnnuiL suggested changes Aug 15, 2023
View reviewed changes
Copy link
Contributor

@EnnuiL EnnuiL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass

build-logic/src/main/java/qsl/internal/Versions.java Outdated Show resolved Hide resolved
library/block/block_extensions/src/main/resources/quilt_block_extensions.accesswidener Outdated
Copy link
Contributor

@EnnuiL EnnuiL Aug 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to be noted: QuiltMC/quilt-mappings#469 has broken the "all blocks are in net/minecraft/block" scheme, meaning that the buildscript should be fixed in order to account for that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

EnnuiL
EnnuiL suggested changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@EnnuiL EnnuiL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass; third incoming later;
There's still a failing check; don't worry, you just have to run ./gradlew applyLicenses

build-logic/src/main/java/qsl/internal/Versions.java Outdated Show resolved Hide resolved
LambdAurora
LambdAurora suggested changes Aug 17, 2023
View reviewed changes
build-logic/src/main/java/qsl/internal/analysis/ClassAnalysisUtils.java Outdated Show resolved Hide resolved
...ource_loader/src/main/java/org/quiltmc/qsl/resource/loader/api/QuiltResourcePackProfile.java
@@ -39,4 +40,18 @@ public interface QuiltResourcePackProfile {
default @NotNull ResourcePackActivationType getActivationType() {
return ResourcePackActivationType.NORMAL;
}

static ResourcePackProfile.ResourcePackFactory identityFactory(ResourcePack pack) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this in API?

Copy link
Contributor Author

@TheGlitch76 TheGlitch76 Aug 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved it to API after noticing that the testmods need it. Vanilla does exactly the same thing, but their implementation BuiltinResourcePackProvider#method_52435(ResourcePack) is protected

@TheGlitch76
Copy link
Contributor Author

@EnnuiL - I can't apply licenses because the licenser is completely broken on Windows (QuiltMC/quilt-gradle-licenser#9)

@TheGlitch76
Copy link
Contributor Author

I'll see if I can just only commit the file update that GHA reports it wants.

EnnuiL
EnnuiL suggested changes Aug 19, 2023
View reviewed changes
Copy link
Contributor

@EnnuiL EnnuiL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please address the CI errors

@EnnuiL EnnuiL added library: core Related to the core library. library: block Related to the block library. t: refactor This proposes a refactor. s: waiting for test This pull request is waiting to be tested, the PR cannot be put in FCP until it has been tested. labels Aug 19, 2023
EnnuiL
EnnuiL reviewed Aug 19, 2023
View reviewed changes
.../resource_loader/src/main/java/org/quiltmc/qsl/resource/loader/api/InMemoryResourcePack.java
@@ -38,6 +38,7 @@
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gson.JsonObject;
import com.mojang.logging.LogUtils;
import net.minecraft.SharedConstants;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't forget about checkstyle

@EnnuiL
Copy link
Contributor

EnnuiL commented Aug 25, 2023

As mentioned on the Discord, I have the intention of taking over this PR in order to kickstart progress on QSL 1.20.2

OroArmor
OroArmor requested changes Aug 31, 2023
View reviewed changes
library/core/networking/src/main/java/org/quiltmc/qsl/networking/api/ServerPlayNetworking.java
@@ -254,7 +254,7 @@ public static void send(ServerPlayerEntity player, Identifier channelName, Packe
Objects.requireNonNull(channelName, "Channel name cannot be null");
Objects.requireNonNull(buf, "Packet byte buf cannot be null");

player.networkHandler.sendPacket(createS2CPacket(channelName, buf));
player.networkHandler.(createS2CPacket(channelName, buf));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
player.networkHandler.(createS2CPacket(channelName, buf));
player.networkHandler.getConnection().send(createS2CPacket(channelName, buf));

@OroArmor OroArmor mentioned this pull request Sep 17, 2023
Merged
@TheGlitch76 TheGlitch76 deleted the 1.20.2-port branch May 29, 2024 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EnnuiL EnnuiL EnnuiL requested changes

@OroArmor OroArmor OroArmor requested changes

@LambdAurora LambdAurora LambdAurora requested changes

Assignees
No one assigned
Labels
library: block Related to the block library. library: core Related to the core library. s: waiting for test This pull request is waiting to be tested, the PR cannot be put in FCP until it has been tested. t: refactor This proposes a refactor.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TheGlitch76 @EnnuiL @LambdAurora @OroArmor