Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add inverted suit #2968

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

IceSelkie
Copy link

@IceSelkie IceSelkie commented Sep 10, 2024

Inverted Suit as proposed in #2937 (and also one of the many suits proposed in #1720 (Ash)).

The inverted suit should play when you try discard, and discard when you try to play.

This seems to work fairly well for the client side view, however a few issues have been found when I was testing it with some friends.

  • Bottom Deck Play option doesn't work yet. It would need to be possible to discard from bottom deck to be able to play an inverted card.
  • Reading from the database will double the inversion, causing the replay to be invalid and cut off part way through.
  • NEW: Merge/rebase onto the new main, since relevant code changed.

Todo (beyond fixing above)

  • Maybe change the animation to be like misplay for discards, and a misplay but in the other direction for successful plays.

Will not do (see Zamiell's comment below):

  • Interactions with determental characters may cause issues.

Where I am hooking in to make the play/discard swap might not be the best place. If someone who knows the code better knows a better place to put this in, do let me know.

@Zamiell
Copy link
Collaborator

Zamiell commented Sep 27, 2024

i wouldn't worry about interactions with detrimental characters, they will probably be deleted in the long term, as they are too much of a maintenance burden

@IceSelkie
Copy link
Author

Original post edited to reflect that detrimental characters won't be considered, and that a merge/rebase needs to be done to make it mergeable again.

@ricardodd2
Copy link

ricardodd2 commented Sep 28, 2024

detrimental character will probably be delated

I would be kinda sad for it to be completely delated. Maybe make it as an option only for no var/make it a variant

@IceSelkie
Copy link
Author

IceSelkie commented Oct 13, 2024

Merged to match main; fixed to pass CI/lint; made bottom-deck-plays also allow discards for inverted games.

https://github.com/IceSelkie/hanabi-live/actions/runs/11310399814

The main functionality is working, so this can probably be merged now.

A few improvements could be made in the future, and I don't know if I'll have much time to investigate these:

  • Inverted plays/discards could use a new better animation/sound so they look/sound distinct from other plays were it not inverted
  • Bottom-Deck Discard is currently allowed user-side on all games, and is checked server-side if the game has inverted or not, giving a warning if it shouldn't be allowed. (Previously, the dragged bottom-deck card would float above discard pile without discarding, playing, returning, or giving a warning)

@IceSelkie IceSelkie marked this pull request as ready for review October 13, 2024 02:09
@Zamiell
Copy link
Collaborator

Zamiell commented Oct 14, 2024

i think it should be some other color instead of black?
black denotes 1 of each

@IceSelkie
Copy link
Author

Oh right, I meant to change that. I'll can go add a new Color of "Inverted". (Though it does works fine now and look how I want; I'm not sure if the word "black" or "K" show up anywhere at the moment, and you can't get any black in the same game as inverted, since I only added "Inverted (5 suits)" and "Inverted (6 suits)" as of now)

@Zamiell
Copy link
Collaborator

Zamiell commented Oct 14, 2024

by the way, why not inverted 3 suits and 4 suits?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants