At minimum, use the same type for types::ReEntryInfo as for structure::ReEntryInfo, but probably just unify to a single type.
Currently there are two different ReEntryInfo types, one that's used for structure templates and the other that is used for tournament Events.
The biggest difference is the structure has a last_re_entry_level field which is a LevelNumber and the Event one has a last_re_entry_round which is a BettingRoundOrdinal. That difference is annoying, but is due to the fact that currently, when creating a tournament, the client doesn't have enough information to access the round names, so instead we use "First Level", "Second Level", etc. even though the true name of the level might be "Level 1A", "Level 1B", etc. This GitHub issue is not about that annoyance.
For no good reason, types::ReEntryInfo has a re_entries field that is a NonZeroU8, whereas structure::ReEntryInfo has max_n_re_entries as an Option<NonZeroU8>, with None meaning that there is no limit to the number of re-entries, which sort of makes sense, if 255 isn't enough, make it unlimited, but that also is kind of crazy in that won't 255 be enough?
My plan has always been to have cleaned this up when we put structures into the database, and we've done OK with this goofy difference, but I'm creating this issue now because I had forgotten some of the particulars and had to refresh my memory when I added structure sheet support (#1439) and i figure I might as well document the difference and the need for unification here, while it's still fresh in my mind.
For the structure sheet support, I'm just using a trait to unify the semantics, which is fine.
At minimum, use the same type for
types::ReEntryInfoas forstructure::ReEntryInfo, but probably just unify to a single type.Currently there are two different
ReEntryInfotypes, one that's used for structure templates and the other that is used for tournamentEvents.The biggest difference is the structure has a
last_re_entry_levelfield which is aLevelNumberand theEventone has alast_re_entry_roundwhich is aBettingRoundOrdinal. That difference is annoying, but is due to the fact that currently, when creating a tournament, the client doesn't have enough information to access the round names, so instead we use "First Level", "Second Level", etc. even though the true name of the level might be "Level 1A", "Level 1B", etc. This GitHub issue is not about that annoyance.For no good reason,
types::ReEntryInfohas are_entriesfield that is aNonZeroU8, whereasstructure::ReEntryInfohasmax_n_re_entriesas anOption<NonZeroU8>, withNonemeaning that there is no limit to the number of re-entries, which sort of makes sense, if 255 isn't enough, make it unlimited, but that also is kind of crazy in that won't 255 be enough?My plan has always been to have cleaned this up when we put structures into the database, and we've done OK with this goofy difference, but I'm creating this issue now because I had forgotten some of the particulars and had to refresh my memory when I added structure sheet support (#1439) and i figure I might as well document the difference and the need for unification here, while it's still fresh in my mind.
For the structure sheet support, I'm just using a trait to unify the semantics, which is fine.