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

Add useResolvedOwners option to swap the owner in member accesses with the resolved class #90

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add useResolvedOwners option to swap the owner in member accesses with the resolved class #90

wants to merge 2 commits into from

Conversation

sfPlayer1
Copy link
Collaborator

Fixes #87 and supersedes #89 with a more extensive approach that covers fields and interface methods as well since those could suffer from the same issue. It also makes the new behavior opt in (for now)

@sfPlayer1 sfPlayer1 mentioned this pull request Mar 14, 2022
Closed
@sfPlayer1 sfPlayer1 force-pushed the useResolvedOwners branch 2 times, most recently from ea14cb9 to f15781a Compare March 14, 2022 18:54
MiniDigger
MiniDigger reviewed Mar 14, 2022
View reviewed changes
src/main/java/net/fabricmc/tinyremapper/TinyRemapper.java
/**
* Whether to use the resolved owner class instead of the current class for member accesses.
*/
public Builder useResolvedOwners(boolean value) {
Copy link

Choose a reason for hiding this comment

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

since we only use the CLI, could you add a new CLI option for this, please? Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure, on it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done :)

@sfPlayer1 sfPlayer1 force-pushed the useResolvedOwners branch 2 times, most recently from 19fa561 to b0bee1f Compare March 14, 2022 19:22
@MiniDigger
Copy link

Upon testing I get a different error now:

Stacktrace 
java.lang.VerifyError: Bad invokespecial instruction: interface method reference is in an indirect superinterface.
Exception Details:
  Location:
    net/minecraft/world/level/ICombinedAccess.a(Lnet/minecraft/core/BlockPosition;Lnet/minecraft/world/level/block/entity/TileEntityTypes;)Ljava/util/Optional; @3: invokespecial
  Reason:
    Error exists in the bytecode
  Bytecode:
    0000000: 2a2b 2cb7 001a b0                      

	at net.minecraft.data.worldgen.features.EndFeatures.<clinit>(EndFeatures.java:18)
	at net.minecraft.data.worldgen.features.FeatureUtils.a(FeatureUtils.java:23)
	at net.minecraft.data.RegistryGeneration.a(RegistryGeneration.java:86)
	at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:721)
	at net.minecraft.data.RegistryGeneration.<clinit>(RegistryGeneration.java:85)
	at net.minecraft.core.IRegistry.<clinit>(IRegistry.java:291)
	at net.minecraft.server.DispenserRegistry.a(DispenserRegistry.java:67)
	at net.minecraft.server.Main.main(Main.java:106)
	at org.bukkit.craftbukkit.Main.main(Main.java:293)

we believe this is similar to what we fix for fields here https://github.com/PaperMC/paperweight/blob/489ab48d996f47d10d2e835e0f08c8716df41c5c/paperweight-lib/src/main/kotlin/tasks/FixJarForReobf.kt#L132
but we will investigate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiniDigger MiniDigger MiniDigger left review comments

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

Successfully merging this pull request may close these issues.

Wrong remap of super method with local static method of same name
2 participants
@sfPlayer1 @MiniDigger