dansanderson / picotool

Tools and Python libraries for manipulating Pico-8 game files. http://www.lexaloffle.com/pico-8.php
MIT License
367 stars 46 forks source link

luamin flag to preserve property names #82

Closed dansanderson closed 2 years ago

dansanderson commented 2 years ago

luamin renames not only names of variables but also names of properties of objects stored in variables referenced with dot syntax: var.prop might become a.b. In Lua, it is also possible to refer to a property with indexing syntax, where the property name is a runtime expression: var["prop"], or even var["pr" + x]. When a PICO-8 cart uses both dot syntax and indexing syntax to refer to the same property, when luamin renames the dot syntax is breaks the runtime indexing. There's no way to programmatically transform the indexing code to work with the new name.

It is rare (and, IMO, poor style) for Lua code to use both dot syntax and indexing syntax for the same property, but it can be useful in some PICO-8 applications. Carts that use this technique need property renaming disabled, at least for the specific properties referred to both ways. luamin could just never rename properties to solve for this edge case, but at the significant expense of carts that only refer to a property with dot syntax: var.some_long_prop_name becomes a.some_long_prop_name.

The next best thing would be to allow the user to disable property renaming with a flag to the tool: p8tool luamin --no-rename-properties foo.p8 (This was proposed in PR 54 as a luamin2 variation, though it works better as a flag.) This might be the most practical solution.

A refinement might be to allow the user to provide a list of property name patterns to preserve (because the indexing technique is most useful for property names that conform to a pattern), either as an external file or with tool hints in the source itself: -- @proppattern elem\d+ would preserve x.elem1, abc.elem987, etc. and allow for x["elem" + n].

It is provably impossible to detect when indexing a variable would refer to a specific property in all cases. Consider: x.prop = 1; y = x; y.prop = 2 Or: x.prop = 1; y.obj = x; z = "obj"; y[z]["prop"] = 2 It may be possible to detect some simpler cases, such as indexing only used with constant expressions. I doubt this would be worth the effort for PICO-8 applications.

dansanderson commented 2 years ago

For anyone looking for this feature, there's now a partial solution: luamin supports a --keep-names-from-file=<filename> argument that takes a list of names from a text file and preserves those names while renaming others. There's also a --keep-all-names flag.

I'm currently thinking that the best solution to the property (attribute) renaming edge case is for the name list to support patterns. That way code that uses both var.prop1 and var["prop" + n] could just add prop* to the name list. I think this is better than a flag that preserves all attribute names. It requires the developer to be explicit about which names to exempt, but captures the major use cases of dynamic attribute indexing that I can think of.

dansanderson commented 2 years ago

I've decided not to support a luamin flag that exempts all properties. Any project that refers to the same property with dot notation and index notation is already dealing with a fixed quantity of properties that the author can put in a file. It's a bit more to ask of authors that are doing this, but I feel like it's enough of a solution and there's not enough added benefit to fully implement something that exempts all property names and nothing else.

I'm not going to do patterns in --keep-names-from-file for now. It wouldn't be too difficult but I think we've got a workable solution as is.