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

Add general parameter naming #9

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

Conversation

JoJoDeveloping
Copy link

This extends the old constructors.txt file in favor of a more general one that allows naming parameters for all kind of methods.
Along this exporting data in the old format is disabled, while importing is retained, but converted to the new format.

Copy link

@JDLogic JDLogic left a comment

Choose a reason for hiding this comment

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

Except for the two issues mentioned, this looks good.


if ((mn.access & Opcodes.ACC_STATIC) == 0)
{
isStatic = true;

This comment was marked as resolved.

if (name.matches("func_\\d+_.+")) // A srg name method params are just p_MethodID_ParamIndex_
nameFormat = "p_" + name.substring(5, name.indexOf('_', 5)) + "_%s_";
else if (name.equals("<init>")) // Every constructor is given a unique ID, try to load the ID from the map, if none is found assign a new one
nameFormat = "p_i" + Constructors.INSTANCE.getID(className, desc, types.size() > 1) + "_%s_";
else

This comment was marked as resolved.

private boolean isSynthetic(MethodNode mn){
if ((mn.access & Opcodes.ACC_SYNTHETIC) != 0) return true;

//check for special case pursuant to JLS 13.1.7
Copy link
Author

@JoJoDeveloping JoJoDeveloping Aug 28, 2019

Choose a reason for hiding this comment

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

@JoJoDeveloping
Copy link
Author

I was about to add special handling for lambds$funcname$x synthetic methods, but they are already assigned srg names in previous steps, so I undid my changes.

@LexManos
Copy link
Member

What is the change in the output data?
Show examples/documentation of the new format.
How much does it change the download size of the standard distributions?
What do you mean by "all kinda of methods"
I have ideas on what you mean, but you should post explicit documentation.

@JoJoDeveloping
Copy link
Author

This replaces the old constructors.txt file, which consisted of lines assinging "srg"-like IDs to constructors so that their params could be named even though the actual method name must always be "". Lines looked like this:
1005 net/minecraft/client/Minecraft$3 (Lnet/minecraft/client/Minecraft;)V

Since the last update caused lots of methods to "lose" their obfuscation, causing their params to become unrenameable, I added this PR, which extends the constructors format (renaming it to params.txt), to not only contain entries for constructors, but also for any other method that is not assigned a srg name.
This deletes the cmdline argument "--ctrOut", and adds "--prm" and "--prmOut", which can be used to set the import and export files for the new params.txt format. "--ctr" is retained and will import the old constructors.txt format to be exported with --prmOut.

The new params.txt file looks like this:

e51806 net/minecraft/util/math/ChunkPos$1 tryAdvance (Ljava/util/function/Consumer;)Z
i1005 net/minecraft/client/Minecraft$3 <init> (Lnet/minecraft/client/Minecraft;)V
s51832 net/minecraft/realms/RealmsMth sin (F)F

The format consists of a number and a prefix char, which is i for initializers so that the param srg names stay the same. s is used for static methods, and is important since the static_methods.txt list does not contain non-srged methods, yet tools need to know if a method is static since that changes the parameter id (the x in p_s12345_x starts at 0 for the first parameter).
Other methods simply have an 'e'.

This results in names for parameters like p_e51806_1, p_i1005_1 or p_s51832_0. Note that for p_i1005_1 is the same srg name as that what would have been generated by the old system.

As for download size change, the current (1.14.4) constructors.txt is 748683b, while the params.txt for 1.14.4 would be 963324b. The mcp_config zip would increase from 644820b to 677592b (with zip -9), which is an increase of 5.1%.

"All kind of methods" means that this tool generates such an "parameter srg base name" for all methods that are not A) synthetic B) already assigned a srg name.

An alternative to this I considered was adding the param base name to the main tsrg file, which would make the whole export data smaller, but require a rewrite of every tool working with tsrg files. Since neither me nor anyone else wants to do that I used this approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants