Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/548/display/redirect

More debug information seems like a good idea. Though I'm wondering, doesn't that slow things down to print debug information? (There shouldn't be any errors in the first place)

Benefits of the patch:

• less error-prone debugging: It is easy to mix up things when debugging as you pointed out in this trac ticket today.
• less time-consuming: One also has to either use an IDE that displays the enum values or compute them manually. There is a number of state variables with similar names which can mislead debugging developers.
• simple code: Since the map is directly below the enum, it seems to be hard to forget the // Keep this in sync with foo part.

Disadvantages of the patch:

• more error-prone code: It's feasible to mismatch string with enums when maintaining.
• slightly more maintenance work, more hunks in patches, more code to learn before comprehending it for beginners

So it seems like a tradeoff of limited advantages and disadvantages.
Given the time that I already spent on matching the numbers with enum constants,
chosing the code strategy where one should optimize the code for progressive revisioning,
maybe this could be considered doable if you want to spend the time on this rather than other things.
But do we really benefit from paying the cost or is it sufficient if we debug correctly when we debug?

In D1517#61919, @elexis wrote:

Benefits of the patch:

• less error-prone debugging: It is easy to mix up things when debugging as you pointed out in this trac ticket today.
• less time-consuming: One also has to either use an IDE that displays the enum values or compute them manually. There is a number of state variables with similar names which can mislead debugging developers.
• simple code: Since the map is directly below the enum, it seems to be hard to forget the // Keep this in sync with foo part.

Disadvantages of the patch:

• more error-prone code: It's feasible to mismatch string with enums when maintaining.
• slightly more maintenance work, more hunks in patches, more code to learn before comprehending it for beginners

So it seems like a tradeoff of limited advantages and disadvantages.
Given the time that I already spent on matching the numbers with enum constants,
chosing the code strategy where one should optimize the code for progressive revisioning,
maybe this could be considered doable if you want to spend the time on this rather than other things.
But do we really benefit from paying the cost or is it sufficient if we debug correctly when we debug?

Good analysis.

In D1517#61919, @elexis wrote:

• more error-prone code: It's feasible to mismatch string with enums when maintaining.

There will happen nothing bad if someone forgets to add a now enum member to the stringmap.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/552/display/redirect

So do I get a "yes" for the approach?

There will happen nothing bad if someone forgets to add a now enum member to the stringmap.

If one entry was renamed but the map wasn't updated that doesn't throw errors, but it's still a bug.
It were really better to keep the single-source-of-truth pattern and just derive the strings from the primary source (perhaps deriving the enum from that primary source too).

It should be possible to have something like this:

MACRO(
// Comment 1
NSS_state1,
// Comment 2
NSS_state2,
...
)

and the macro would then create the enum and the std::map.

Then the try-catch block can be removed, the LOGERROR dupe can be removed, the slightly increased maintenance and errorproneness were avoided.

The macro in NetMessages.h apparently does something similar to what we want here, resulting in the C++ variable names in the mainlog.html file:
<p>Net client: Received message CAuthenticateResultMessage { m_Code: 0, m_HostID: 12, m_Message: Logged in } of size 29 from server</p>

In D1517#63044, @elexis wrote:

The macro in NetMessages.h apparently does something similar to what we want here, resulting in the C++ variable names in the mainlog.html file:
<p>Net client: Received message CAuthenticateResultMessage { m_Code: 0, m_HostID: 12, m_Message: Logged in } of size 29 from server</p>

Yeah, but that only works by putting every state list in its on file.

Currently I see only 3 possibilities:

• https://stackoverflow.com/a/49595815 But then the state names wouldn't be C++ code
• Same as in NetMessages.h, but then we'd need one extra file for each enum
• Current solution, but there we have duplication

How about this? https://stackoverflow.com/questions/201593/is-there-a-simple-way-to-convert-c-enum-to-string/238157#238157

(There also are D492 / D1578 doing things to the network FSM code)

In D1517#72556, @elexis wrote:

How about this? https://stackoverflow.com/questions/201593/is-there-a-simple-way-to-convert-c-enum-to-string/238157#238157

That's more or less the same as my first bullet point.

(There also are D492 / D1578 doing things to the network FSM code)

As far as I can see they don't change anything with the enums, except that the MessageTypes are moved into a separate file.

@elexis Do you have any preferred solution?
I'd like to have this feature but don't care too much about its way of implementation.