Support unpicking of fields

[liach]

This technically shouldn't be too hard; those fields can just be treated as a union of getter and setter methods in pretty much all senses. And yes, we indeed might have fields using constants/flags that are public. Just for consideration.

[Earthcomputer]

I looked into this (I actually have it half implemented locally, lol). The biggest hurdle is that it would need a v3 unpick file format. Is this okay with everyone?

[modmuss50]

v3 is ok, would be a good chance to look at other changes to the format as well. I know @sfPlayer1 has some ideas around it.

[liach]

Imo we can just bump the format version without concerns. It's no different from a build number. If maintenance cost for older version is high, you can even probably drop older version (like v1) support

[sfPlayer1]

I'd like to eventually see the java member associated data ("target_method") become part of the regular enigma/tiny-2 data to avoid the two parallel structures and associated porting difficulties.The part that defines constants sources ("constant") is a bit more difficult since it may be backed by a non-mc class, but with the recent obfuscation changes it may make sense to just create enigma files for the yarn target class?

[liach]

Interesting idea! I don't know if my understanding here is correct:
So currently unpick has constants defined as literal/flag, and the definition and actual replacements are linked by ids.

We might have the definition and target entries both exist as subsections of method/field (much like how comments exist in current enigma/tiny)? That actually sounds quite good.

If we do need outsourced (i.e. declared in separate constant jar, etc.) constants, we might just add to an extra file in enigma or to tiny header. But this need isn't urgent given most constants are already provided by mojang since 21w13a.

[Daomephsta]

We might have the definition and target entries both exist as subsections of method/field (much like how comments exist in current enigma/tiny)? That actually sounds quite good.

You are proposing removing constant groups? Defining the constants in the target method definition would improve conciseness, but it's not a suitable replacement for constant groups. Constant groups allow a set of constants to be defined once, then referenced by many targets. Use cases like biome_ids.unpick would be tedious to write and verbose otherwise.

[sfPlayer1]

I'd keep the groups basically exactly as they are, just embedded into the normal enigma/v2 data.

In v2 you'd e.g. have something like

c net/minecraft/entity/passive/CatEntity
  m (I)V setCatType
    p 0 catType
      unpick-type cat_types
[...]
c net/fabricmc/yarn/constants/CatTypes
  f Lsomething; TABBY
    unpick-constant cat_types

[liach]

Ah, so player's example gladly is my understanding. Imo the constant group ids we definitely have to keep; otherwise we cannot distinguish between 100 different types of 0.

[Juuxel]

Hmm, would Player's example also work for third-party constants (like GLFW key codes etc)?

[liach]

Hmm, would Player's example also work for third-party constants (like GLFW key codes etc)?

i suggest using tiny v2 header meta (might need to update) and extra file for enigma

[Earthcomputer]

Specifically, the problem would come where a third party library accepts parameters or returns values from a constant group. Not with constants that happen to be from libraries.

Another problem with player's approach is what if we want to extend again later? What if, for example, we want to be able to specify a way to say that the length of an array is from a constant group?

[sfPlayer1]

I wouldn't put it in some magic header metadata values, but additional member "mapping" entries, just with empty or unity dst name columns. They don't necessarily need to do mapping:

c no/oh/SomeLibClass   (empty dst name columns)
  m (I)V someMethod
    p 0 catType
      unpick-type cat_types

or

c no/oh/SomeLibClass no/oh/SomeLibClass no/oh/SomeLibClass (3 times the same)
  m (I)V someMethod
    p 0 catType
      unpick-type cat_types

The Yarn repo/Enigma could then either use a complementary tiny file containing these or use regular enigma mapping files following the same principle.

Wrt further extension there should always be a way to denote a concrete target in the mapping files, be it the existing local variable syntax in the v2 spec or something entirely new. It's not really different from the unpick files, only more integrated with the mapping data. Not every file would necessarily have mappings then, could be purely unpick with no-op mappings.

[Earthcomputer]

I'd also like to take this opportunity to make a couple of breaking changes to unpick's API if that's okay with everyone. I can create a new issue to describe them