Support for comments on conditions.

[redwizard42]

Adds support for comments on conditions. Uses Haskell style block comments ( {- to start, -} to end) for the serialized form since it is unlikely a human will enter those character comments in an editor and for ease of parsing and pattern machine for filtering comments out of logic (see guidance, below). Comments are to be located at the end of a condition's definition.

Considerations/Guidance for consumers:

• Editors should not serialize any portion if the comment is an empty/null string (including the comments start and end character sequences of the comment).
• Editors should filter out the comment block begin and end character sequences or replace it with something else that can be deserialized back.
• Editors should replace new lines with \r\n when serializing and convert \r\n to new lines when deserializing.
• Comments do not affect functionality and as such, it would be best for clients that do not edit assets to have the comment blocks filtered out by RAWeb. As such it would be reasonable to have editor clients be able to request that comments be included when retrieving assets. (This is outside the scope of this PR)

[redwizard42]

I am uncertain if using rc_alloc_str is appropriate to pulling the string out, but it did result in the expected assignment. Will work on fixes for the workflow check failures in the meantime.

[Jamiras]

I have strong opinions against including the comments directly in the serialized data.

The 64KB field size limitation in the database already restricts the size of an achievement, and the largest, most complex achievements are the ones that would most benefit from this. Additionally, something like 0xH1234=5{-was 0xH1234=6-}_0xH2345=0 would be confusing to the regex split code used to display logic on the server.

This should be handled like code notes - as separate data that's only retrieved in clients that care about it.

[redwizard42]

I have strong opinions against including the comments directly in the serialized data.

The 64KB field size limitation in the database already restricts the size of an achievement, and the largest, most complex achievements are the ones that would most benefit from this. Additionally, something like 0xH1234=5{-was 0xH1234=6-}_0xH2345=0 would be confusing to the regex split code used to display logic on the server.

This should be handled like code notes - as separate data that's only retrieved in clients that care about it.

That is fair and I had similar initial thoughts, though I also thought it would lead to concise comments being used. I went ahead and did this implementation as a first attempt anyway since it's come up in several conversations I had recently and comments tied to conditions in some fashion seemed to be preferable.

Do we have a potential pathway were we could have notes 1:1 with a condition without affecting the serialized string? (or minimally affecting it)

I also debated comments-as-a-condition where it's a comment flag with an index to a comment that could be separate, though I worried about how that would present in the current grid of the asset editor.

[Jamiras]

You also need to consider the editor side of things. How would the comments be displayed/edited in the achievement editor?

Clearly having the comments embedded in the achievement would be the most fool-proof way to keep them in sync with the achievement logic, but it complicates pretty much every other aspect of dealing with the achievement logic. Storage, transmission, processing (whether it be client side or filtered out on the server).

I'd like more information on the need for this functionality as well. A code note should be sufficient for documenting a single logical condition. Having a single comment for the entire achievement would probably be the easiest and least disruptive solution, but saying "conditions 14-18 do this" doesn't work very well if a new condition has to be added to something before 14. It would probably be best to have a field on the achievement itself that is just a mapping of comments to conditions, managed by the editor.

I think this feature needs to be discussed more before any attempts are made to implement it. Perhaps we can take the discussion back to https://github.com/RetroAchievements/RAIntegration/discussions/887 and come up with a reasonable solution there.

[redwizard42]

I think this feature needs to be discussed more before any attempts are made to implement it. Perhaps we can take the discussion back to RetroAchievements/RAIntegration#887 and come up with a reasonable solution there.

Partly its when we end up with funky-looking logic or chains that are hard to decipher through inspection, it'd be nice to have some form of comment near the code to explain its nuances; many times we can do that with code notes, but I'd argue that code notes aren't a great place to talk logic-implementation .

Yep. I've been pondering a variety or presentations. I'm working on getting some experience in the codebase to work towards solutions towards the memory structures of Metroid Prime, where a few ideas would benefit from having explanations nearby, rather than at the top of the achievement (though perhaps that would be enough for many cases)

Spitballing some design and other ideas with wes. Will try to refine some ideas and then hop over to the discussion.