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

fix inputGameJars ObjectShare mutating after being set #876

Merged
merged 3 commits into from Jan 15, 2024
Merged

fix inputGameJars ObjectShare mutating after being set #876

merged 3 commits into from Jan 15, 2024

Conversation

tildejustin
Copy link
Contributor

No description provided.

modmuss50
modmuss50 reviewed Dec 24, 2023
View reviewed changes
minecraft/src/main/java/net/fabricmc/loader/impl/game/minecraft/MinecraftGameProvider.java Outdated
@@ -243,7 +243,7 @@ public boolean locateGame(FabricLauncher launcher, String[] args) {
// expose obfuscated jar locations for mods to more easily remap code from obfuscated to intermediary
ObjectShare share = FabricLoaderImpl.INSTANCE.getObjectShare();
share.put("fabric-loader:inputGameJar", gameJars.get(0)); // deprecated
share.put("fabric-loader:inputGameJars", gameJars);
share.put("fabric-loader:inputGameJars", new ArrayList<>(gameJars));
Copy link
Member

Choose a reason for hiding this comment

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

I would use unmodifiableList

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrapping gameJars with Collections.unmodifiableList doesn't fix the issue that the underlying list is mutated in place. I could make the new list unmodifiable if that's what you mean.

@modmuss50
Copy link
Member

doesn't fix the issue that the underlying list is mutated in place
What issue is this? What problem does this PR solve?

@thecatcore
Copy link
Contributor

It seems like the shared list ends up with intermediary remapped jar in the end when accessing it.
Making the shared list immutable should prevent this from happening.

@sfPlayer1
Copy link
Contributor

What modifies the underlying jar? I don't think it's Fabric Loader, but a misbehaving mod?

@thecatcore
Copy link
Contributor

It's loader's doing https://github.com/FabricMC/fabric-loader/blob/master/minecraft/src/main/java/net/fabricmc/loader/impl/game/minecraft/MinecraftGameProvider.java#L328

@tildejustin
Copy link
Contributor Author

tildejustin commented Dec 24, 2023 •
edited

image
sometime after gameJars is put into the objectshare, it is changed to hold the intermediary jar instead of the obsfucated one (exactly what catcore said). Sorry, I put this image in Fabriccord and assumed people would have seen it, bad assumption 😅.

@sfPlayer1
Copy link
Contributor

Please add a comment explaining that it gets modified subsequently by remapping

@tildejustin
Copy link
Contributor Author

done

@modmuss50 modmuss50 merged commit 1030e56 into FabricMC:master Jan 15, 2024
3 checks passed
AlexIIL pushed a commit to QuiltMC/quilt-loader that referenced this pull request Feb 14, 2024
@tildejustin @AlexIIL
Fix inputGameJars ObjectShare mutating after being set
3389b7c 
Upstream from fabric-loader:
FabricMC/fabric-loader@1030e56
(Original PR: FabricMC/fabric-loader#876 )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfPlayer1 sfPlayer1 sfPlayer1 approved these changes

@modmuss50 modmuss50 modmuss50 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tildejustin @modmuss50 @thecatcore @sfPlayer1