Hanabi-Live / hanabi-live

A web server that allows people to play Hanab, a cooperative card game of logic and reasoning.
https://hanab.live
GNU General Public License v3.0
179 stars 118 forks source link

Remarks regarding user_stats table / variant id discussion #2420

Closed DrAnax closed 1 year ago

DrAnax commented 2 years ago

Related: #2350

The table contains invalid data and it should be recreated.

  1. hanab.live/scores/melwen reports total games 11583 whereas adding up the total games by expanding all the rows it adds up to 11586. The difference exists because the code checks at the user_stats table where there are variants under the said player's id which do not exist in games table (ie player number: 5977, variant_id 1645)
  2. Table user_stats also contains columns best_scoreX_mod (x being 2 to 6) which contain data not used in any part of the code. Additionally, the columns contain false data: the stats are updated at the end of each game and, since there's a single row for every unique pair of [player_id, variant_id], the last game's bitmask modifiers overwrites the previous value.

Suggestions:

  1. Remove redundant columns best_scoreX_mod (x being 2 to 6)
  2. Update the code, reflecting the removal of those columns
  3. Recreate the user_stats data via external set of SQL queries
Zamiell commented 2 years ago

afaik the "best_scoreX_mod" column is necessary to distinguish between a maximum score with no external options and a maximum score with e.g. the "All or Nothing" option, which trivializes the game

Zamiell commented 2 years ago

if it is not being used by the code, that then is almost surely a bug

DrAnax commented 2 years ago

if it is not being used by the code, that then is almost surely a bug

My bad it does. Still I have this:

SELECT distinct user_stats.variant_id FROM user_stats WHERE NOT EXISTS (SELECT variant_id FROM games WHERE games.variant_id = user_stats.variant_id)

variant id 1645 : Synesthesia & Black (4 suits) appears on user_stats (21 games) but there's no game with that variant

Zamiell commented 2 years ago

i dont know why that would be

DrAnax commented 2 years ago

variant ID 1645 was Extremely Ambiguous & Prism (5 Suits) removed by commit c1c7c87b2b37a24664adc284c554f1134b0d11aa (12/6/2020) and reintroduced as Synesthesia & Black (4 Suits) by commit be2f256de0fda454190e842e903a913db2437a99 (10/29/2021).

My guess is that people played some games with Extremely Ambiguous & Prism (5 Suits), then the variant (as well as the games) was deleted and then reintroduced.

Zamiell commented 2 years ago

oh i see

Zamiell commented 2 years ago

there should be some kind of process that redoes stats when new variants are introduced/deleted

DrAnax commented 2 years ago

... and perhaps have IDs of deleted variants removed forever and not reintroduced later on?

Zamiell commented 2 years ago

if the stats table is updated properly after a variant deletion, then there should be no reason to not reuse old variant IDs, right?

DrAnax commented 2 years ago

if the games are also deleted, right

Zamiell commented 2 years ago

tangent: we should also talk about converting variant_id column to some kind of new format that is a string representing the suits and variant modifiers

DrAnax commented 2 years ago

why not simply have a table variants with id and the rest of the data into separate columns? why have the variants and their data only in txt/json files?

DrAnax commented 2 years ago

simple, undigested idea tables variants id int data json

DrAnax commented 2 years ago

or maybe id, name, data

Zamiell commented 2 years ago

because i want to move away from having variant IDs at all, such that people can create tables with arbitrary suits

DrAnax commented 2 years ago

So, there will be something like different rows for selection, ie

row 1: suits : normal 3-6 XOR ambiguous 3-6, black, pink, null etc row 2: additional rules: up & down, synesthesia, critical fours etc

Games won't start very quickly; it's like putting a child in a candy store and giving him your credit card; also it's going to be a nightmare creating sane stats/history/report pages.

For sure there has to be a limited selection of suggestions for new players

Zamiell commented 2 years ago

also it's going to be a nightmare creating sane stats/history/report pages.

the idea is that this part of the website is phased out - only very specific suit combinations and variants are tracked for the purposes of "stat completion"

Zamiell commented 2 years ago

row 1: suits : normal 3-6 XOR ambiguous 3-6, black, pink, null etc row 2: additional rules: up & down, synesthesia, critical fours etc

that's one way you could do it, you could also store it inside of a single string e.g.

"RYGBM-T" would represent the 5 suits, red yellow green blue rainbow, and then a hyphen, with a letter representing each variant modifier, with T standing for Throw it in a Hole

Zamiell commented 2 years ago

For sure there has to be a limited selection of suggestions for new players

the ui to create a "custom" variant would be gated behind the "more options" button in exactly the same way that the 1000s of extra variants currently are

DrAnax commented 2 years ago

that's one way you could do it, you could also store it inside of a single string e.g.

I was referring to the UI

Zamiell commented 2 years ago

oh, the ui will be complicated yeah, im not worried about that part yet

DrAnax commented 2 years ago

I have started a spreadsheet which calculates the new variant id (string), given the base suits, the extra suits, the variation rules or the special variation. Please have a look and comment wherever appropriate.

https://docs.google.com/spreadsheets/d/1H6F0ZZENWsUV4oZpLFBxDBcBPN69MlcHT8L5vE8AKYA/edit

Zamiell commented 2 years ago

ok so i talked with DrAnax a bit today in DMs and it sounds like we should start by coming up with a unique two-letter ID for every suit in suits.json

unless anyone has any better ideas? there are multiple ways to go about this

DrAnax commented 2 years ago

The worksheet is done

DarthGandalf commented 2 years ago

The problem with this format is extensibility. When new variants are added, it needs to fit somehow to the new format. Currently every new variant is possible, because it's just increasing the number. If you change the ids to the proposed format, it will be hard to add new variants which don't fit the scheme.

For the <suits> part, consider delimiter-based format instead: R+B+OM-5u is also easily parsed. As you can guess, it's "null fives & omni (3 suits)". The benefit is that we can add suit modifiers there easily. E.g. R+Y+G/UD+B/UD+P/R+T/R would be "1st 2 suits 1-5, 2nd 2 suits either direction, last 2 suits 5-1 (aka reversed)". Another suit modifier could be crit 4 of that particular suit. Or there could be more, none of which are implemented yet. Having always 2 char per suit is not enough.

N1trate commented 2 years ago

I agree with Darth. It looks very interesting to make our own variants on-the-fly, and I thought that it was the goal. I don't think the cost (complexity) of the string for the new variant-id is that high compared to what was proposed? (May be wrong!) Wasn't it already proposed to be a full listing a suits and per-game modifier? At first, would it be possible to only allow identical suit modifications (i.e. UoD everywhere, not only 2 out of 5 suits) to ease the transition?

While thinking about it, it could go even one step further and remove all of the 1oE suits (Black/Dark /Gray /Cocoa Rainbow). Aren't they just a variation of another suit? So R+Y+G+B+OM/1 (for 1oE), R+Y+G+B+OM/U (for Unique) R+Y+G+B+OM/D (for Dark) or R+Y+G+B+OM/1oE would be the current "Dark Omni (5 Suits)". If we're doing that, we would need to be able to apply multiple suit-modifier to get what we know as Dark Rainbow Reversed (5 Suits) (R+Y+G+B+RA/D,R). It would also allow us to play Critical 4 + Null-Ones (R/C4,N1+Y/C4,N1+G/C4,N1+B/C4,N1+P/C4,N1), which is not currently possible. That opens the door for incompatible modifier like asking for a suit to be both UoD and Reversed, which may be painful to manage and a good reason to have a maximum of a single modifier per suit, and therefore not making 1oE a suit-modifier but individual suits like they are now.

I also think that if we allow multiple suit-modifier, we need another separator. I gave the previous example to follow Darth's schema, but I think I would prefer to separate suits with a comma or semi-colon instead of the +, to be able to use the + for multi-modifier.

Regarding that Critical 4 + Null-Ones example, maybe a modifier after the - that is not a game modifier (i.e. CP/Cow&Pig, AC/AlternatingClues...) would apply to all non-conflicting suits, so it could be written as R+Y+G+B+P-C4,N1.

This whole thing also calls for duplication like Reverse R/R+Y+G+B+P, R+Y/R+G+B+P, R+Y+G/R+B+P... but if we don't do /missing anymore, is that a problem?

Who's interested in a R/N1,Y,G,B,P-R1? (Rainbow-Ones except Red ones which are null)

Good luck. :grin:

Edit: Modified T(eal) as being the fifth color to P(urple) to make them more valid examples/questions.

Zamiell commented 2 years ago

I agree with Darth. It looks very interesting to make our own variants on-the-fly, and I thought that it was the goal.

yes that is the goal, the first baby step is converting variant numbers to some other kind of more-specific-representation in the database

Zamiell commented 2 years ago

remove all of the 1oE suits (Black/Dark /Gray /Cocoa Rainbow)

i dont think so, these have explicit pips, names, colors, and so forth

Zamiell commented 2 years ago

I would prefer to separate suits with a comma or semi-colon instead of the +, to be able to use the + for multi-modifier.

type out a few examples, starting small and increasing in complexity

Zamiell commented 2 years ago

This whole thing also calls for duplication like Reverse R/R+Y+G+B+T, R+Y/R+G+B+T

the idea is that suits that are not properly sorted would be validated/illegal

DarthGandalf commented 2 years ago

I also think that if we allow multiple suit-modifier, we need another separator.

I was assuming that multiple modifiers would be appended to the suit, eg R/R/C4 to make the red suit be reversed and have critical 4 (critical 2?)

But the exact syntax of this could be with a separate separator like with your example, sure. As long as it's easy to extend the syntax for future variants.

Initially the set of available variants would simply replicate what we have now anyway

Romain672 commented 2 years ago

Here is my proposition, longer, but more obvious imo: 3 Suits would be:

Black (5 Suits) would be

Light-Pink-Ones (3 Suits) would be:

Critical 4 + Null-Ones (5 Suits):

Reversed prism (3 Suits):

Rainbow-Ones except Red ones which are null (5 Suits):

The drawback is that you can easily find similar variant noted differently and it's longer. But the clues given could be checked during the variant: the server search for this card exactly, if he finds it, it look for corresponding color/number, if he didn't find it, he search for color or number modifier, and if he didn't find it, it take the default thing about that color. I think 5 ; r1:n,rr-,-11 would be 5 Suits but one r1 is brown, one r1 is normal, and one r1 is white.

DarthGandalf commented 2 years ago

@Romain672 can you describe more formally what are different parts of your syntax? What are the things between commas, colons and semicolons?

How does Up/Down look like?

DarthGandalf commented 2 years ago

Looks like you're only describing some parts of rules which clue touches which card. But variants have more: which color to draw the card in, which pip image to use for the suit, which color to show in the UI when giving the color clue, etc

5 ; t:11111,n,n

you don't want the black suit to look like teal?

DarthGandalf commented 2 years ago

Here's the plan:

  1. add some code to scripts/typescript/create_variants_json.ts to assign the new id to all variants to a new field in the existing variants.json
  2. write a parser which would parse the id and verify that what it parsed matches what is specified in variants.json
  3. server and the DB can be updated to use the new id.
  4. server can be updated to use the parser and remove variants.json.

Since the server is in go, I won't write the parser which can be plugged to the server, but I can write a standalone script just for the test. And I can update create_variants_json.ts to output the new ids in the format I proposed

PS. the standalone test script is not perfect because the implementation of the parser in go needs to have tests of its own to check that they match

Zamiell commented 2 years ago

you missed a step where old games in the DB have to be updated with the new DB field somehow

Zamiell commented 2 years ago

otherwise it seems good

DarthGandalf commented 2 years ago

I meant it as part of "server and the DB can be updated to use the new id."

DarthGandalf commented 2 years ago

I also missed a step where there's a new UI for custom variant builder, but that's part of step 4

Zamiell commented 2 years ago
  /**
   * The suit ID is a two character string:
   * - Normal suits have a single capital letter. (e.g. "R" for "Red")
   * - Special suits have a two character ID with a lowercase letter. (e.g. "Bk" for "Black")
   * - Combination suits have a four character ID. (e.g. "BrRa" for "Muddy Rainbow")
   * - Dark suits have a capital D prefix. (e.g. "DRa" for "Dark Rainbow")
   * - Ambiguous suits have a capital A prefix and a number suffix. (e.g. AR1 for "Tomato")
   * - Very Ambiguous suits have a capital V prefix and a number suffix.
   *   (e.g. "VR1" for "Tomato VA")
   * - Extremely Ambiguous suits have a capital E prefix and a number suffix.
   *   (e.g. "EB1" for "Ice EA")
   * - Dual-Color suits have a capital D prefix. (e.g. "DRY" for "Orange D", "DRB" for "Purple D",
   *   and "DRY2" for "Orange D2")
   */
Zamiell commented 2 years ago

the issue hasn't been resolved

Romain672 commented 2 years ago

Here is the 'suffixes' each variant proposed and live got: https://docs.google.com/document/d/18bJffn2Ir1tEox395gUQKskzGIDggqmZ8WpwluyYJ5A/edit

Krycke commented 1 year ago

I was just thinking about this one again. I wonder if we need to do the strings super super small, we could do something like: [ { c:p, o:r, i:{1:3,2:2,3:2,4:1,5:1} }, {}, {}, {}, { c:r, i:b } ]

Which would mean first color being purple with order in reverse and 4's being unique. Second, third and forth color is the next default color not already used, and the 5th color is rainbow with the cards being all unique. We could of course add alot of short hands if we want like u4 for unique 4's. But in the end these strings aren't supposed to we written or read manually, but generated through a UI.

For statistical purposes we could just create a smaller string with all esthetics removed that doesn't matter for the stats (i.e. color) and all default values removed, and then order everything alphabetical.

This should create an easy way to modify everything and don't lock ourselves in a to narrow box. There is no real reason to make the string completely unreadable.

Zamiell commented 1 year ago

I wonder if we need to do the strings super super small

well it saves space in the database for N games, so its ideal to make it as concise as possible

Zamiell commented 1 year ago

@DrAnax looks like the google doc is deleted now, do you have a backup?

Zamiell commented 1 year ago

Ok I have been heavily working on this over the past 2 days:

  /**
   * The suit ID is a two character string:
   * - Normal suits have a single capital letter. (e.g. "R" for "Red")
   * - Special suits have a two character ID with a lowercase letter. (e.g. "Bk" for "Black")
   * - Combination suits have a four character ID. (e.g. "BrRa" for "Muddy Rainbow")
   * - Dark suits have a capital D prefix. (e.g. "DRa" for "Dark Rainbow")
   * - Ambiguous suits have a capital A prefix and a number suffix. (e.g. AR1 for "Tomato")
   * - Very Ambiguous suits have a capital V prefix and a number suffix. (e.g. "VR1" for "Tomato
   *   VA")
   * - Extremely Ambiguous suits have a capital E prefix and a number suffix. (e.g. "EB1" for "Ice
   *   EA")
   * - Dual-Color suits have a capital D prefix. (e.g. "DRY" for "Orange D", "DRB" for "Purple D",
   *   and "DRY2" for "Orange D2")
   */

You can see all of the variant-specific suffixes here: https://github.com/Hanabi-Live/hanabi-live/blob/main/packages/data/src/enums/VariantModifier.ts