saluk / PyWright-godot

An implementation of the PyWright Casemaker in the Godot engine
7 stars 2 forks source link

Strategy to make command processing more consistent, while still supporting old scripts #51

Open saluk opened 2 months ago

saluk commented 2 months ago

Where commands rely on different default behavior, we can make them more consistent. To support old scripts, they can be updated by replacing the commands with versions that use the new logic but with explicit behavior instead of default values. For example:

fade name=blah vs delete name=blah

In PyWright, delete will delete the topmost object with the name, while fade will delete all of the objects with the name. This is confusing and leads to bugs. It's also very hard in PyWright to know if you have deleted all of the objects with the given name, because you would have to call delete each time (there is no way to delete all of them).

GodotWright could add the arguments name_top and name_all to distinguish these behaviors.

A conversion tool, or when loading code known to be from a pywright game (and not designed in godotwright), could modify existing calls like this:

fade name=black
delete name=blah

would become:

fade name_all=black
delete name_top=blah
saluk commented 2 months ago

Another example would be if we want to add quotes around arguments to get rid of the weird spacing rules that sometimes apply and sometimes don't.

setvar text "This is some text"

We would have to: accept text that isn't quoted, you just can't have spaces in it (bash style)

saluk commented 1 month ago

Another example: {p} inside of text is affected by the {spd} value. It's a little unintuitive, a pause should have a consistent length. A complicated fix would be to alter the math on import:

"{spd0.5}Some text{p60}Some more text"

... could convert to

"{spd0.5}Some text{p120}Some more text"

This has 2 downsides. One, is that we have to parse the text character by character to determine the speed at the time. Two, it's ambiguous when looking at it that the text has been modified. Not all of the examples in this issue are clear in this way, however this is the most hidden.

We could introduce a backwards compatible pause function or an argument to the pause function, replacing all previous usages with the backwards compatible version. Something like:

{spd0.5}Some text{spdpause 60}Some more text"

Also worth pointing out the "conversion" doesn't necessarily have to happen in a way that's nice for humans to read, or even well documented.