{"action":"created","issue":{"url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/issues/3055","repository_url":"https://api.github.com/repos/MinecraftForge/MinecraftForge","labels_url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/issues/3055/labels{/name}","comments_url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/issues/3055/comments","events_url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/issues/3055/events","html_url":"https://github.com/MinecraftForge/MinecraftForge/pull/3055","id":163463647,"number":3055,"title":"Add additional field map for deobfuscation to track overrides ","user":{"login":"Mumfrey","id":1147071,"avatar_url":"https://avatars.githubusercontent.com/u/1147071?v=3","gravatar_id":"","url":"https://api.github.com/users/Mumfrey","html_url":"https://github.com/Mumfrey","followers_url":"https://api.github.com/users/Mumfrey/followers","following_url":"https://api.github.com/users/Mumfrey/following{/other_user}","gists_url":"https://api.github.com/users/Mumfrey/gists{/gist_id}","starred_url":"https://api.github.com/users/Mumfrey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Mumfrey/subscriptions","organizations_url":"https://api.github.com/users/Mumfrey/orgs","repos_url":"https://api.github.com/users/Mumfrey/repos","events_url":"https://api.github.com/users/Mumfrey/events{/privacy}","received_events_url":"https://api.github.com/users/Mumfrey/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/labels/Bug","name":"Bug","color":"e11d21"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-07-01T19:52:23Z","updated_at":"2016-07-20T00:45:33Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/pulls/3055","html_url":"https://github.com/MinecraftForge/MinecraftForge/pull/3055","diff_url":"https://github.com/MinecraftForge/MinecraftForge/pull/3055.diff","patch_url":"https://github.com/MinecraftForge/MinecraftForge/pull/3055.patch"},"body":"This pull request represents a possible solution to #3043. I have attempted to keep the changes to a minimum to achieve clarity, however the reasons for the specific changes made may not be particularly obvious so I will attempt to describe the nature of the problem and the solution implemented here.\r\n\r\nApologies if this seems a little Peter-and-Jane, I wanted to make it perfectly clear what I'm trying to achieve so that any potential pitfalls of the fix can be identified more easily.\r\n\r\n### The Problem\r\n\r\nConsider two **vanilla** classes, `Foo` and `Bar`. `Foo` extends `Bar` in this example. In the diagram below, diamonds reprent **field declarations** and circles represent **field accesses**. The arrows represent the accesses to the fields.\r\n\r\n![](http://eq2.co.uk/pr/obf/obf1.png)\r\n\r\nEach **vanilla** class has mappings for its fields and methods, these are stored in associated **raw mappings** which are themselves stored in a map of classname -> raw map.\r\n\r\nAt runtime, since field names need to be resolved in super classes as well, the raw maps are merged into a single map which is then used to map field declarations and field accesses within the class. It's important to note that because the raw map contains entries for fields in `Foo`, if name conflicts occur with the superclass these are still properly resolved in the merged map. In the example, the field `Foo.b` which hides the field `Bar.b` is still remapped correctly.\r\n\r\n![](http://eq2.co.uk/pr/obf/obf2.png)\r\n\r\nAfter deobfuscation, all the fields and field accesses have the correct names:\r\n\r\n![](http://eq2.co.uk/pr/obf/obf3.png)\r\n\r\nLet's now consider a **non-vanilla** class `MyCustomClass` which extends the vanilla class `Bar`:\r\n\r\n![](http://eq2.co.uk/pr/obf/obf4.png)\r\n\r\nSince `MyCustomClass` has no raw map, it can *only* resolve field names using the merged table. This normally works as intended, except in the situation that `MyCustomClass` contains a field with the same name as an obfuscated field in the superclass.\r\n\r\n![](http://eq2.co.uk/pr/obf/obf5.png)\r\n\r\nThis is a problem for the field `b` in this example, since the merged map contains a mapping for this field, and there is no local *raw* map to correct the problem:\r\n\r\n![](http://eq2.co.uk/pr/obf/obf6.png)\r\n\r\nWe can resolve this problem by synthesising the raw map for our derived class, populating the raw map first with entries for the fields which exist in the class.\r\n\r\n![](http://eq2.co.uk/pr/obf/obf7.png)\r\n\r\nOnce this is done, the merged map contains both entries for the local fields, effectively mapped to themselves (preventing them being erroneously remapped), and the unhidden field renames from the superclass are still resolved correctly.\r\n\r\n![](http://eq2.co.uk/pr/obf/obf8.png)\r\n\r\n### The Solution\r\n\r\nSince in the `ClassVisitor` pattern, member fields are \"visited\" before the methods, we have the opportunity to populate the synthetic raw map (or insert entries into a pre-existing one) in the manner described above by overriding `VisitField` and have it perform the duty of populating the map.\r\n\r\nThis is achieved by the following:\r\n\r\n* A helper method is added to `FMLDeobfuscatingRemapper`, `getRawFieldMap` which retrieves or creates the raw field map for a class\r\n\r\n* A method `mapMemberFieldName` is also added, which behaves like `mapFieldName`, but **only** resolves field names in the raw map (if one exists) and immediately *stores* the mapping once resolved. This has the effect of creating the synthetic entries mentioned above.\r\n\r\n* `FMLRemappingAdapter` is then updated to include a `visitField` overload which calls `mapMemberFieldName` instead of `mapFieldName`\r\n\r\nThe proposed changes are included in this PR.\r\n"},"comment":{"url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/issues/comments/233808300","html_url":"https://github.com/MinecraftForge/MinecraftForge/pull/3055#issuecomment-233808300","issue_url":"https://api.github.com/repos/MinecraftForge/MinecraftForge/issues/3055","id":233808300,"user":{"login":"killjoy1221","id":3409962,"avatar_url":"https://avatars.githubusercontent.com/u/3409962?v=3","gravatar_id":"","url":"https://api.github.com/users/killjoy1221","html_url":"https://github.com/killjoy1221","followers_url":"https://api.github.com/users/killjoy1221/followers","following_url":"https://api.github.com/users/killjoy1221/following{/other_user}","gists_url":"https://api.github.com/users/killjoy1221/gists{/gist_id}","starred_url":"https://api.github.com/users/killjoy1221/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/killjoy1221/subscriptions","organizations_url":"https://api.github.com/users/killjoy1221/orgs","repos_url":"https://api.github.com/users/killjoy1221/repos","events_url":"https://api.github.com/users/killjoy1221/events{/privacy}","received_events_url":"https://api.github.com/users/killjoy1221/received_events","type":"User","site_admin":false},"created_at":"2016-07-20T00:45:32Z","updated_at":"2016-07-20T00:45:32Z","body":"Until this gets pulled, I'm going to be using a workaround I discovered. Unfortunately, it is not very desirable. I'm going to be distributing two separate jars. One for using with forge and one for not. Forge one will be reobfuscated using searge names while the other is fully obfuscated."}}
{"actor":{"display_login":"killjoy1221"}}