Fix 3 problems with plSubWorldMsg.
#1485
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The messages in Responder commands are reused, so it's important to clear out the receivers each iteration of the Responder command. Otherwise, we are basically leaking memory in a way that asan won't detect.
From TPotS to MOUL, the avatar's physics related SDLs were removed from the avatar state descriptor and stashed into the avatarPhysical descriptor. So, when a subworld change happens, we need to syncrhonize avatarPhysical, not avatar.
This is the most controversial change. The onus for this is a report that, when using Korman's Change Subworld Message node, when a player uses an activator that sends this message in a responder, all players change to that subworld. This is objectively incorrect. The code is currently written to send `plSubWorldMsg` to the local player. Unfortunately, there is currently no way to prevent a responder running on remote clients when its clickable is used without redirecting through some kind of hack Python script. The rationale here is that changing subworlds modifies a player's avatarPhysical SDL. So, really, we probably only want to be sending `plSubWorldMsg` to the local player from a Responder. Responders are primarily a zero-code artist logic tool, so they shouldn't have to care about that level of SDL detail. Further, there is precedent for major avatar/player related actions to be rejected from non-local sources. Namely, all camera messages are restricted in the same way in Responders. The alternate to this change would be to send the `plSubWorldMsg` to the player who triggered the responder i.e. `fPlayerKey`. We will need to validate all existing (read: Cyan) Responders to see which approach they expect.
dpogue
approved these changes
Sep 16, 2023
|
Update: I checked all of Cyan's responders, and none of them send subworld messages, so there is no danger of this breaking Cyan content. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The impetus for this change is that Korman's new
Change Subworldmessage node can cause all players in the Age to change to that subworld. The rationale here is that changing subworlds modifies a player's avatarPhysical SDL. So, really, we probably only want to be sendingplSubWorldMsgto the local player from a Responder. Responders are primarily a zero-code artist logic tool, so they shouldn't have to care about that level of SDL detail. Further, there is precedent for major avatar/player related actions to be rejected from non-local sources. Namely, all camera messages are restricted in the same way in Responders.As a bonus, I fixed a memory leak that wouldn't be detected by asan and fixed a logic error with the subworld SDL synch.