Add enum value for decapitation game mode

[Askaholic]

Needed for FAForever/fa#5347

Can we change the enum to be the actual string names of the victory conditions without it messing up the historical data? Seems pretty bad to me to be using stringified integers.

[Brutus5000]

According to this SO article it should work since it's only single digit.

Needs a matching change in the API, but I'm all in for cleaning types.

[Askaholic]

I think simply updating the column with the new enum values would work since the index is related to the definition order. I don’t think any row updates are needed since we don’t need to change the order of the enum.

https://mariadb.com/kb/en/enum/
Funny enough, this guide specifically discourages the use of numerals as enum values because it can lead to confusing behavior and kindof defeats the purpose of using an enum in the first place.

[Askaholic]

@Brutus5000 is this good to go?

[Brutus5000]

Totally forgot about this one. I want to test this on the test server to ensure it works with server and api.

[Brutus5000]

@Askaholic short update: this breaks the API. It now always returns null for victoryCondition 😞
Will have to figure out why.

[Brutus5000]

@Askaholic Actually the problem is different. Your change drops the values, even though it is non null.

I ran it against the test-data.sql
Before, the gameType is 0.

After the statement it is in an inconsistent state!

MariaDB [faf]> select * from game_stats;
+----+---------------------+---------+--------------+----------+---------+------+-------+-----------+----------+------------------+
| id | startTime           | endTime | replay_ticks | gameType | gameMod | host | mapId | gameName  | validity | replay_available |
+----+---------------------+---------+--------------+----------+---------+------+-------+-----------+----------+------------------+
|  1 | 2024-04-04 16:42:19 | NULL    |         NULL |          |       6 |    1 |     1 | Test game |        0 |                0 |
+----+---------------------+---------+--------------+----------+---------+------+-------+-----------+----------+------------------+
1 row in set (0.000 sec)

I prepared the required changes in the API but you will have to find a working migration.

[Askaholic]

Ok, I’ll have to look into it more then

[Brutus5000]

Setting this to "changes requested" for correct status reporting

[Askaholic]

Changed it to the approach described here: https://dba.stackexchange.com/questions/6547/can-i-rename-the-values-in-a-mysql-enum-column-in-one-query

I was hoping you could do the rename without having to update the rows in the table, but it does not appear to be possible without some hacky stuff. The game stats table is pretty big so it will have to update a lot of rows.

[Brutus5000]

I'm not sure if we can safely remove the old enum values ever.
I would propose to create a new nullable column victory_condition for the new enum.
Then update into this with a single update with case whens (multiple updates will increase the runtime I would assume).
Then make the column non-nullable and drop the old column.

[Askaholic]

I'm not really sure what safety concerns you are having and how making an entirely new column would address those, but I'm also all for renaming the column to victory_condition as I think that's a much better name.

Were you thinking of dropping the column in a separate migration/PR?

[Brutus5000]

Because every enum member is mapped to an id, what happens if you drop the first value from the enum list? It's actually not documented. could be that all values get "shifted" because just the enum<->id mapping shifts.

Anyway. I thought about keeping the old column for potentially rolling back. But that wouldn't work because the new version of the python client would need to populate both. But if it populates both we can't remove the old column either. So let's just drop it right after. In case of a rollback we do a whole db rollback.

[Askaholic]

Yes, so I actually was wondering the same thing before I made this PR and I actually tested it in my faf-db container locally with a scratch table. The ids are updated to match the order of the enum. So after the migration is complete the mapping would be:

DEMORALIZATION -> 1
DOMINATION -> 2
ERADICATION -> 3
SANDBOX -> 4
DECAPITATION -> 5

It seems that Maria will update the underlying ID values when you alter the column so that the ids match the order of the enum definition and any string values are still preserved. That’s why my first attempt at this PR didn’t work, because the old string values were wiped away so Maria didn’t know what the int values should be and replaced them all with nulls.