0ad/0ad
0ad/0ad
4
5
Fork 14
Code Issues 1.5k Pull Requests 28 Actions Projects Releases Wiki Activity

Reduce time needed for STUN #6953

Closed
Dunedan wants to merge 0 commits from Dunedan/0ad:reduce-time-for-stun into main
pull from: Dunedan/0ad:reduce-time-for-stun
merge into: 0ad:main
Conversation 7 Commits - Files Changed -
Dunedan commented 2024-08-22 08:15:43 +02:00
Member
Copy Link

In my tests this reduced the time necessary for starting to host a game
or joining a hosted game by ~180ms.

Continuation of https://code.wildfiregames.com/D5321

In my tests this reduced the time necessary for starting to host a game or joining a hosted game by ~180ms. Continuation of https://code.wildfiregames.com/D5321
Dunedan added the
Theme
Multiplayer Lobby
 label 2024-08-22 08:15:43 +02:00
Dunedan added 1 commit 2024-08-22 08:15:45 +02:00
Reduce time needed for STUN
Some checks failed
0ad-checkrefs/pipeline/pr-main This commit looks good
Details
0ad-windows/pipeline/pr-main This commit looks good
Details
0ad-macos/pipeline/pr-main There was a failure building this commit
Details
0ad-linux/pipeline/pr-main This commit looks good
Details
5e5187f6f4 
In my tests this reduced the time necessary for starting to host a game
or joining a hosted game by ~180ms.
Dunedan force-pushed reduce-time-for-stun from 5e5187f6f4 to 5609ec878c 2024-08-22 08:17:54 +02:00 Compare
Stan commented 2024-08-22 09:44:20 +02:00
Owner
Copy Link

Looks alright to me. Maybe num_msg could not be abbreviated also max_tries sounds like it should be camelCase. Any way to test this on a bad network?

Looks alright to me. Maybe num_msg could not be abbreviated also max_tries sounds like it should be camelCase. Any way to test this on a bad network?
Itms commented 2024-08-22 13:18:20 +02:00
Owner
Copy Link

Continuation of https://code.wildfiregames.com/D5321

You can post a message on that diff, such as "Continuation over at #6953", and close the diff.

> Continuation of https://code.wildfiregames.com/D5321 You can post a message on that diff, such as "Continuation over at https://gitea.wildfiregames.com/0ad/0ad/pulls/6953", and close the diff.
Dunedan commented 2024-08-22 13:45:43 +02:00
Author
Member
Copy Link

Maybe num_msg could not be abbreviated

IMO number_of_messages would sound pretty strange.

also max_tries sounds like it should be camelCase.

In the C++ code I assume, because in the config file Snake Case seems to be common?

Any way to test this on a bad network?

I haven't tested it with bad network, but as this PR is just changing the frequency of checking for responses, without changing the overall time, I don't expect any negative impact, no matter how good or bad network connectivity is.

> Maybe num_msg could not be abbreviated IMO `number_of_messages` would sound pretty strange. > also max_tries sounds like it should be camelCase. In the C++ code I assume, because in the config file Snake Case seems to be common? > Any way to test this on a bad network? I haven't tested it with bad network, but as this PR is just changing the frequency of checking for responses, without changing the overall time, I don't expect any negative impact, no matter how good or bad network connectivity is.
phosit commented 2024-08-22 20:21:58 +02:00
Member
Copy Link

Maybe num_msg could not be abbreviated

IMO number_of_messages would sound pretty strange.

Maybe message_count

> > Maybe num_msg could not be abbreviated > > IMO `number_of_messages` would sound pretty strange. Maybe `message_count`
πŸ‘ 1
Stan commented 2024-08-22 20:31:49 +02:00
Owner
Copy Link

Yeah I meant camelCase for C++

Yeah I meant camelCase for C++
Dunedan added 1 commit 2024-08-23 08:19:07 +02:00
Use camel case for variable names
All checks were successful
0ad-checkrefs/pipeline/pr-main This commit looks good
Details
0ad-freebsd/pipeline/pr-main This commit looks good
Details
0ad-windows/pipeline/pr-main This commit looks good
Details
0ad-linux/pipeline/pr-main This commit looks good
Details
0ad-macos/pipeline/pr-main This commit looks good
Details
1f42adee75
Dunedan commented 2024-08-23 08:20:36 +02:00
Author
Member
Copy Link

I updated the variable names to use camel case. As for the abbreviations, I believe they are fine as they are.

I updated the variable names to use camel case. As for the abbreviations, I believe they are fine as they are.
Stan approved these changes 2024-08-23 08:24:12 +02:00
Stan left a comment
Owner
Copy Link

I don't really have any more objections.

I don't really have any more objections.
Dunedan force-pushed reduce-time-for-stun from 1f42adee75 to 8519eb9b86 2024-08-23 21:03:05 +02:00 Compare
Dunedan commented 2024-08-23 21:05:29 +02:00
Author
Member
Copy Link

Merged as 8519eb9b86

Merged as 8519eb9b86
Dunedan closed this pull request 2024-08-23 21:05:30 +02:00
Some checks are pending
0ad-freebsd/pipeline/head This commit looks good
Details
0ad-checkrefs/pipeline/head This commit looks good
Details
0ad-checkrefs/pipeline/pr-main Build queued...
Details
0ad-linux/pipeline/pr-main Build queued...
Required
Details
0ad-macos/pipeline/pr-main Build queued...
Details
0ad-windows/pipeline/head This commit looks good
Details
0ad-windows/pipeline/pr-main Build started...
Details
0ad-freebsd/pipeline/pr-main This commit looks good
Details
0ad-linux/pipeline/head This commit looks good
Required
Details
0ad-macos/pipeline/head This commit looks good
Details
checkrefs / checkrefs *
Required
pre-commit / build *
Required

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Stan
Labels
Clear labels   
Closed
Duplicate

  
Closed
Fixed

  
Closed
Invalid

  
Closed
Needs info

  
Closed
Won't fix

  
Closed
Works for me

  
Difficulty
Hard

  
Difficulty
Medium

  
Difficulty
Simple

  
Needed for Beta

   
Needs Design Input

   
Needs Info

   
Pathfinding
  
Priority
1: Release Blocker

  
Priority
2: Must Have

  
Priority
3: Should Have

  
Priority
4: Nice To Have

  
Priority
5: If Time Permits

  
Regression
  
Theme
AI

  
Theme
Art & Animation

  
Theme
Atlas editor

  
Theme
Build & Packages

  
Theme
Core engine

  
Theme
Internationalization & Localization

  
Theme
Maps

  
Theme
Multiplayer Lobby

  
Theme
Music & Sound FX

  
Theme
Network

  
Theme
Non-game systems

  
Theme
Simulation

  
Theme
UI & Simulation

  
Theme
UI – Game setup

  
Theme
UI – In-game

  
Theme
UI – Miscellaneous

  
Theme
Website & Forum

  
Type
Defect

  
Type
Enhancement

  
Type
Task

No Label
Closed
Duplicate
Closed
Fixed
Closed
Invalid
Closed
Needs info
Closed
Won't fix
Closed
Works for me
Difficulty
Hard
Difficulty
Medium
Difficulty
Simple
Needed for Beta
Needs Design Input
Needs Info
Pathfinding
Priority
1: Release Blocker
Priority
2: Must Have
Priority
3: Should Have
Priority
4: Nice To Have
Priority
5: If Time Permits
Regression
Theme
AI
Theme
Art & Animation
Theme
Atlas editor
Theme
Build & Packages
Theme
Core engine
Theme
Internationalization & Localization
Theme
Maps
Theme
Multiplayer Lobby
Theme
Music & Sound FX
Theme
Network
Theme
Non-game systems
Theme
Simulation
Theme
UI & Simulation
Theme
UI – Game setup
Theme
UI – In-game
Theme
UI – Miscellaneous
Theme
Website & Forum
Type
Defect
Type
Enhancement
Type
Task
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: 0ad/0ad#6953
No description provided.
Delete Branch "Dunedan/0ad:reduce-time-for-stun"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?