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.

Sign up for GitHub

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

Jump to bottom

Holopad networking rework #34112

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Holopad networking rework #34112

wants to merge 7 commits into from
+92 −252

Conversation

chromiumboy
Copy link
Contributor

@chromiumboy chromiumboy commented Dec 29, 2024 •
edited
Loading

About the PR

This PR changes how data needed by holopad holograms is networked and managed

Why / Balance

The original approach for networking data for the holopad hologram sprites was client-authoritative. The server requested the the client for the player's sprite data, and then relayed this data to the other clients. There was consequently a possibility that cheat clients could relay sprite data of their own choosing. Here the networking has been reworked so that holopad users (and their equipped/held item) are added to the expanded PVS instead. Required data is now relayed via component states, rather than networked events, so holograms should have their proper appearance in replays. Support for clothing displacement maps has also been added

These changes were made to address multiple issues raised in #33986

Technical details

N/A

Media

Requirements

Breaking changes

The following potentially breaking changes have been made to holopads

  • The LinkedHolopad field has been removed from HolopadHologramComponent (it was redundant and unused)
  • HolopadHologramVisualsUpdateEvent, PlayerSpriteStateRequest and PlayerSpriteStateMessage have been removed (they are no longer required with the changes made to Server.HolopadSystem and Client.HolopadSystem)

Changelog

N/A

@github-actions github-actions bot added size/M Denotes a PR that changes 100-999 lines. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Dec 29, 2024
@chromiumboy chromiumboy mentioned this pull request Dec 29, 2024
Open
8 tasks
@chromiumboy chromiumboy added T: Bugfix Type: Bugs and/or bugfixes P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. T: Cleanup Type: Code clean-up, without being a full refactor or feature D2: Medium Difficulty: A good amount of codebase knowledge required. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. A: General Interactions Area: General in-game interactions that don't relate to another area. T: Of Admin Interest Type: Affects administration work a lot, and might require admins to weigh in on and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Dec 29, 2024
metalgearsloth
metalgearsloth requested changes Jan 13, 2025
View reviewed changes
Content.Client/Holopad/HolopadSystem.cs Outdated Show resolved Hide resolved
Content.Server/Holopad/HolopadSystem.cs Outdated Show resolved Hide resolved
Content.Server/Holopad/HolopadSystem.cs Outdated Show resolved Hide resolved
Content.Server/Holopad/HolopadSystem.cs Outdated Show resolved Hide resolved
@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Jan 14, 2025
@chromiumboy
Copy link
Contributor Author

Has been approved by sloth on Discord

Just double checking that everything is working correctly before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metalgearsloth metalgearsloth Awaiting requested review from metalgearsloth

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
A: General Interactions Area: General in-game interactions that don't relate to another area. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted size/M Denotes a PR that changes 100-999 lines. T: Bugfix Type: Bugs and/or bugfixes T: Cleanup Type: Code clean-up, without being a full refactor or feature T: Of Admin Interest Type: Affects administration work a lot, and might require admins to weigh in on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chromiumboy @metalgearsloth