Rework special node button dictionary

[CelticMinstrel]

The special node button_dict (in special.cpp) encodes what sort of button should be placed on each special node field and what kind of picker should appear when that button is pressed. It's encoded as 2-dimensional arrays with each ASCII character having a different meaning. It's quite compact and works just fine, but actually editing it to fix issues or add a new type is rather painstaking.

I'm trying to think of a better way to do it that's easier to edit while not being overly verbose. This is in service of eventually adding several new picker types (including some dynamic pickers), as well as a "simple toggle" case which replaces the entire field with an LED.

Ultimately what it needs to do is to populate a node_properties_t struct for each possible special node type. Directly declaring the array of node_properties_t would probably be possible but feels like it would be overly verbose.

[NQNStudios]

Oh dear... Somehow I'd never seen this before now.

cboe/src/scenario/special.cpp

Line 583 in e83ad32

static const char*const button_dict[7][11] = {

My gut reaction is that I hate it.

[NQNStudios]

What if the information was encoded as XML, which was written by a WYSIWYG editor showing what the edit special node dialog would look like when the node's definition was parsed?

[CelticMinstrel]

That sounds like extreme overkill for something that's only going to be edited by developers… it's not the most terrible of ideas, but I don't think I like it.

[CelticMinstrel]

I got started on trying to clean this up. The above commit defines the structure of how the information should be in memory, while retaining the magic characters during loading.

Any comments are welcome.

[NQNStudios]

I was baffled recently by trying to parse through this area of code the other day:

3bd4adc#diff-176a6d3d8f719743296a0862725f49ddc4706bde78837e720bec1ab78b9c6200L880

And I can say that it is a lot more readable with enums than with magic characters as the case labels.

[CelticMinstrel]

Does it seem like the above WIP commit might be going in the right direction? (Pay the closest attention to the newly-added file.)

[NQNStudios]

Seems like you accidentally deleted the weight field from item.hpp?

But I really like the way it's going, with the builder pattern. It's much more readable to see which node has which properties.

(The only thing about the builder pattern is, it's only readable if you recognize it & know how it works. When I first saw spells.cpp I was super confused about what it was representing. So, we should make sure to have comments explaining well what the file is doing and how to add new nodes.)

[CelticMinstrel]

Considering various ways of formatting this. The above commit shows "compact" style in specials-general and specials-oneshot, but "verbose" style in specials-affect.

Beyond the difference shown there, there are a few other options:

• I could make .finish() be implicit if I didn't use auto on the declarations (they'd need to be declared as node_property_t).
• I could also avoid auto but still keep .finish().
• Even if not using the "verbose" style, I'd probably break up the longest ones into multiple lines, but not one field per line.

There's no need to respond to this, but if you have any thoughts on the matter at all, feel free to let me know.

[NQNStudios]

I don't think I mind the verbose style. I do think finish() should be implicit.

[CelticMinstrel]

I think that completes the rework of the button dictionary? It looks like the pickers are working correctly still.

[NQNStudios]

It can't really break the game, can it? Just the editor?

[CelticMinstrel]

It can actually break the game, since this structure is also used by the function that parses special nodes from a file.

[CelticMinstrel]

This is closed by #655 / 4174ccd