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

Fixes related to SendTileRectHandler #3063

Merged
merged 3 commits into from
Feb 28, 2025

Conversation

LaoSparrow
Copy link
Contributor

Fixed incorrect validating range in TileRectMatch.MatchRemoval.
Fixed tile rect changes (e.g. turning on and off campfires) are not synced between clients.
Fixed unable to place Hat Rack without permission tshock.ignore.sendtilesquare.

@hakusaro
Copy link
Member

@LaoSparrow is this code from a fork of TShock perhaps? Why did you change the types of Matches etc., to a MatchResult enum in the same commit? I assume this is because you've already done this in a fork and want to upstream these changes...?

@LaoSparrow
Copy link
Contributor Author

The use of MatchResult is to distinguish between these three cases:

  • NotMatched: the tile rect is not matched (e.g. tileid not matched, size not matched etc. ). In this case we will skip to the next match
  • RejectChanges: changes from a player is rejected (e.g. doesn't have a build permission etc. ). In this case the tiles on the server side are unchanged, and we want to revert the changes on the player's client
  • BroadcastChanges: changes from a player is accepted (e.g. normal operations, like turning on or off campfires). In this case the tiles on the server side are changed, and we want to broadcast these changes to other players as well

So the purpose of adding MatchResult is to solve this issue:

Fixed tile rect changes (e.g. turning on and off campfires) are not synced between clients.

@LaoSparrow
Copy link
Contributor Author

and yes I would like to upstream these bugfixes

@hakusaro hakusaro merged commit 8d186bb into Pryaxis:general-devel Feb 28, 2025
8 checks passed
@hakusaro
Copy link
Member

@LaoSparrow
lgtm_trick

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

Successfully merging this pull request may close these issues.

3 participants