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.

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

Methods that override the return type don't get Mapped #14

Closed
MiniDigger opened this issue May 5, 2020 · 4 comments
Closed

Methods that override the return type don't get Mapped #14

MiniDigger opened this issue May 5, 2020 · 4 comments
Labels
bug Something isn't working

Comments

@MiniDigger
Copy link
Member

Example:
public abstract EntityAgeable createChild(EntityAgeable entityageable);
Overridden:

    @Override
    public EntityBee createChild(EntityAgeable entityageable) {
        return (EntityBee) EntityTypes.BEE.a(this.world);
    }

SRG:
MD: net/minecraft/server/EntityAgeable/createChild (Lnet/minecraft/server/EntityAgeable;)Lnet/minecraft/server/EntityAgeable; net/minecraft/world/entity/AgableMob/getBreedOffspring (Lnet/minecraft/world/entity/AgableMob;)Lnet/minecraft/world/entity/AgableMob;

this will remap the method in EntityAgeable, but not in EntityBee, because the return types don't match. Java allows overriding methods with different return types, if the type is a subclass of the OG return type.

This is fixable by adding extra SRGs for the subclasses, but thats rather annoying:
MD: net/minecraft/server/EntityBee/createChild (Lnet/minecraft/server/EntityAgeable;)Lnet/minecraft/server/EntityBee; net/minecraft/world/entity/animal/Bee/getBreedOffspring (Lnet/minecraft/world/entity/AgableMob;)Lnet/minecraft/world/entity/animal/Bee;

@jamierocks jamierocks added the bug Something isn't working label May 8, 2020
jamierocks added a commit that referenced this issue May 8, 2020
@jamierocks
Copy link
Member

jamierocks commented May 8, 2020

To provide some context for the following, this issue is closely related to GH-8. Both issues are effectively issues with Lorenz's implementation of complete (though generics are a slightly extended problem).

This issue can be resolved with the following, and I'll be looking to include this change (or one to similar effect) in an upcoming version of Lorenz (0.5.3) - which will then only nessecitate a version bump here.

// Check if there are any methods here that override the return type of a parent
// method.
for (final Map.Entry<MethodSignature, InheritanceType> method : info.getMethods().entrySet()) {
    final MethodSignature mappingSignature = mapping.getSignature();
    final MethodDescriptor mappingDescriptor = mappingSignature.getDescriptor();
    final MethodSignature methodSignature = method.getKey();
    final MethodDescriptor methodDescriptor = methodSignature.getDescriptor();

    // The method MUST have the same (obf) name, and parameters
    if (!Objects.equals(methodSignature.getName(), mappingSignature.getName())) continue;
    // TODO: handle generic params
    if (!Objects.equals(methodDescriptor.getParamTypes(), mappingDescriptor.getParamTypes())) continue;

    if (mappingDescriptor.getReturnType().isAssignableFrom(methodDescriptor.getReturnType(), provider)) {
        this.methods.putIfAbsent(methodSignature, mapping);
    }
}

edit: see https://github.com/CadixDev/Lorenz/compare/fix/mercury-14

@TheGlitch76
Copy link

Bumping to say Patchwork needs fix/mercury-14 merged for remapping YarnForge

@jamierocks
Copy link
Member

Bumping to say Patchwork needs fix/mercury-14 merged for remapping YarnForge

Yep, I've been wanting to get this through - though I'm still in need of feedback regarding any possible regressions caused by this commit.

By this point, I think I'll merge it later today and make a release though - can always make another release if things break (but hopefully they don't).

@jamierocks
Copy link
Member

Should be resolved by Lorenz 0.5.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants