SerenityOS / serenity

The Serenity Operating System 🐞
https://serenityos.org
BSD 2-Clause "Simplified" License
30.26k stars 3.17k forks source link

Solitaire: Card suits are covered up in stacks #7487

Open MaxWipfli opened 3 years ago

MaxWipfli commented 3 years ago

When cards are stacked in Solitaire, the player cannot see what suit the cards below have (see image below).

image

trflynn89 commented 3 years ago

Just for reference, this is a change I made to fix this last time, before the position of the number / suit was changed: https://github.com/SerenityOS/serenity/commit/cfc3a2ebac3702c5517847a22a75e682aa219e9a

This time, I wouldn't be surprised if bumping the cards down further would necessitate making the window a bit taller.

marius137 commented 3 years ago

instead of making the stack further apart, wouldn't it be better to place the suit next to the number or add an option to right click to see the full card like in kpatience? Example of large stacks and the Right Click thing mentioned

MaxWipfli commented 3 years ago

instead of making the stack further apart, wouldn't it be better to place the suit next to the number or add an option to right click to see the full card like in kpatience?

The problem I see is that Hearts (the other card game) has cards arranged in a way that need the suit to be below the number. Although I still prefer that to right-clicking, maybe we could implement an option into LibCards to specify that.

matthewbjones commented 3 years ago

Changing Solitaire::Game::height from 480 to 546 in Game.h and also changing constexpr StackRules rules_for_type(Type stack_type) in CardStack.h for case Normal from return { 0, 20, 1, 3 } to return { 0, 26, 1, 3 } results in the following:

Screen Shot 2021-05-31 at 10 39 44 PM

So yes, in order to accommodate slightly taller stacks the window height has to increase a bit. (In the screenshot I just generated a full stack in the last position to visualize it) Is increasing the window height undesirable? For what it's worth, the game was playable previously on a 800x600 resolution but now the window is just slightly too tall and the status bar gets cut off.

An alternative solution, as suggested, would be to pass in a "suit orientation" to Card::construct, to tell it which orientation to use (vertical or horizontal), with it defaulting to vertical as it does today, and only when specifying an alternative orientation, e.g. Card::construct(Card::Type::Diamonds, i, false), would result in being able to keep the window and stack spacing the same and seeing something like this:

Screen Shot 2021-05-31 at 11 10 34 PM

I've been working on two other PRs for Solitaire, so have no problem submitting another PR for this issue if there is a preferred approach.

gunnarbeutner commented 3 years ago

Personally I think moving the cards further apart looks nicer than having the cards' suit next to the cards' value. But that's just me. Also, revealing the cards by right-clicking on them would be cool, too. :)