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

Draw offers #39

Merged
Merged

Conversation

this-is-not-available
Copy link
Contributor

Not perfect, but it is a start

@this-is-not-available
Copy link
Contributor Author

this-is-not-available commented Jul 10, 2024

Currently it only changes the dev version of the site. (Someone needs to minify it or some automated system should)

@this-is-not-available
Copy link
Contributor Author

Also users can currently spam draw offers

Copy link
Contributor Author

@this-is-not-available this-is-not-available left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍! I don't know the project completely, so I just try stuff to see what sticks.

@this-is-not-available
Copy link
Contributor Author

Spam prevention currently doesn't work because I forgot to update drawOfferMove in the client and the server after the checks

@Naviary2
Copy link
Member

Naviary2 commented Jul 10, 2024

This is great!!! Nice work. The functionality is there, I was able to successfully conclude the game by draw agreement! I'm going to make a list of things this still needs. Mostly polishing and bug fixes! Others are welcome to help fix these!

Screen Shot 2024-07-10 at 2 15 11 PM Screen Shot 2024-07-10 at 2 16 11 PM
  • The main menu and draw offer button need to be swapped so main menu is always on bottom.
  • The button should not be present in local games.
  • Should not be present in online abortable games (less than 2 moves played)
  • Should not be present after the game is over. Hide it when gamefileutility.concludeGame() is called.
  • If you refresh the page while you have a draw offer it breaks it.
  • We need some form of cap for how often you can send an offer. Maybe once every few moves?

And mostly just a preference, I think it should be less intrusive, maybe don't darken the whole screen, and place the popup in the lower middle of the board, or upper middle.

@this-is-not-available
Copy link
Contributor Author

this-is-not-available commented Jul 10, 2024

Ok! I will start work on this tomorrow. Starting with the limit and local game darkening.

@Naviary2 Naviary2 added enhancement New feature or request modification needed A change needs to be made labels Jul 10, 2024
@this-is-not-available
Copy link
Contributor Author

this-is-not-available commented Jul 11, 2024

And mostly just a preference, I think it should be less intrusive, maybe don't darken the whole screen, and place the popup in the lower middle of the board, or upper middle.

Because I'm lazy (and don't have much time), I decided that players shouldn't be able to make moves, while there is a draw offer.
It isn't server-sided yet or for the offerer, so that needs updating.
But this can change later on, if players or you want it to change.
For now, I'm sticking with this.

I made the background less dark though.

Refreshing the page no longer breaks it (untested), games with <2 moves cant offer draws, the draw offer UI is less dark and can't offer draws after game end.

From what I've tested, it works great!
Addresses Naviary's list of changes.

Co-Authored-By: Naviary <[email protected]>
@Naviary2
Copy link
Member

That's what I mean about being less intrusive, I can only imagine how annoyed people will get if they can essentially pause their opponents game.

I'm thinking no black transparent backdrop, and a popup box either in the lower middle or the corner of the screen that does not interrupt your play.

Don't worry if you don't have time now, take your time. I'd rather this be polished than rushed.

@this-is-not-available
Copy link
Contributor Author

Ok

@Naviary2
Copy link
Member

This is a new design we came up with in the discord discussion

Screen Shot 2024-07-11 at 7 35 48 AM copy

@this-is-not-available this-is-not-available marked this pull request as draft July 11, 2024 14:12
@Naviary2 Naviary2 changed the base branch from main to draw-offers August 9, 2024 05:42
@Naviary2 Naviary2 marked this pull request as ready for review August 9, 2024 05:44
@Naviary2 Naviary2 merged commit 4187a94 into Infinite-Chess:draw-offers Aug 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants