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

refactor(netork)!: rewrite ping logic #5106

Merged
merged 13 commits into from
Jul 15, 2023
Merged

Conversation

skaldarnar
Copy link
Member

@skaldarnar skaldarnar commented Jul 5, 2023

Contains

Started off with some boy-scout rule improvements while looking at the code, and ended in a bigger experiment on consolidating (unneccessary?) complexity in the logic around pings and ping subscriptions.

Also added a bunch of docstrings to improve the comprehensibility of the code.

How to test

Start a multiplayer game with multiple connected clients and ensure that the ping information in the UI still looks reasonable.

Outstanding before merging

n/a

We have introduced type handlers for generic maps some time ago, and there is no need anymore to keep the ping information as separate lists.

Therefore, refactor the internal storage of ping data to use a `Map<EntityRef, Long>`.
@skaldarnar skaldarnar requested a review from jdrueckert July 5, 2023 18:35
@github-actions github-actions bot added the Type: Improvement Request for or addition/enhancement of a feature label Jul 5, 2023
@skaldarnar skaldarnar added the Multiplayer Affects aspects not visible in Singleplayer mode only label Jul 5, 2023
@skaldarnar skaldarnar force-pushed the topic/rewrite-ping-system branch from 56d2dc4 to 59ab883 Compare July 5, 2023 20:08
…into PingComponent

feat(nui): improve online players visuals
@jdrueckert
Copy link
Member

Tests out fine. The only thing not so great (yet) is the formatting in the ping info overlay.
However, that is not blocking and can be improved in a follow-up PR.

@skaldarnar skaldarnar marked this pull request as ready for review July 10, 2023 17:57
@skaldarnar skaldarnar changed the title !feat(netork): rewrite ping logic feat(netork)!: rewrite ping logic Jul 10, 2023
@skaldarnar skaldarnar merged commit 828cd51 into develop Jul 15, 2023
@skaldarnar skaldarnar deleted the topic/rewrite-ping-system branch July 15, 2023 11:55
@skaldarnar skaldarnar changed the title feat(netork)!: rewrite ping logic refactor(netork)!: rewrite ping logic Jul 15, 2023
@skaldarnar skaldarnar added Breaking Change API breaking change requiring follow-up work in dependant areas Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity labels Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change API breaking change requiring follow-up work in dependant areas Multiplayer Affects aspects not visible in Singleplayer mode only Type: Improvement Request for or addition/enhancement of a feature Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity
Projects
Status: Done
Status: No status
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants