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

Automatically suggest adding/removing remap=true on mixin-related annotations #251

Open
JBYoshi opened this issue Jul 4, 2017 · 4 comments
Labels
platform: mixins status: accepted This is a high-priority feature or a reproduced bug type: enhancement

Comments

@JBYoshi
Copy link
Contributor

JBYoshi commented Jul 4, 2017

Could you add a warning for mixin annotations when the remap value of an annotation doesn't match the obfuscation state of the target?

Relevant annotations I've found:
@Mixin
@Accessor
@Invoker
@Shadow
@ModifyConstant
@Inject
@ModifyVariable
@At
@ModifyArg
@ModifyArgs
@Redirect

@JBYoshi
Copy link
Contributor Author

JBYoshi commented Jul 5, 2017

This will also be added to @Overwrite in the future: SpongePowered/Mixin#179 (comment)

@stale
Copy link

stale bot commented Feb 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status: stale label Feb 11, 2018
@DenWav
Copy link
Member

DenWav commented Feb 11, 2018

remap = true is the default.

This should be specific for targetting non-obfuscated targets, without remap = false.

@stale stale bot removed the status: stale label Feb 11, 2018
@DenWav DenWav added the status: accepted This is a high-priority feature or a reproduced bug label Feb 11, 2018
@stephan-gh stephan-gh removed their assignment Jun 28, 2018
@magneticflux-
Copy link

@DenWav From SpongePowered/Mixin#551, annotations may still require an explicit remap=true in some cases even though it is the default.

A good first step for this would be to suppress the "default annotation value" warning and quick fix for the remap field, since it can be misleading. Then, we could work on re-adding an inspection explicitly for the remap that is aware of the unconventional context-sensitive behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: mixins status: accepted This is a high-priority feature or a reproduced bug type: enhancement
Projects
None yet
Development

No branches or pull requests

5 participants