Skip to content

added sv_race_server for race maps & only reload servers when changing tab - #342

Closed
timakro wants to merge 6 commits into
ddnet:masterfrom
timakro:master
Closed

timakro wants to merge 6 commits into
ddnet:masterfrom
timakro:master

Conversation

@timakro

@timakro timakro commented Oct 3, 2015

Copy link
Copy Markdown
Member

Race feature will be used for the new race servers and contains commits:
18d82fa, 7156449 and a22a085

Remove useless variables and only reload servers when changing tab in ingame browser contains commits:
8983b29

@timakro timakro changed the title added sv_race_server for race maps added sv_race_server for race maps Oct 3, 2015
@heinrich5991

Copy link
Copy Markdown
Member

Adding new config options apart from sv_race_server is probably unnecessary.

This changes the behavior to not remove the projectiles on player death for the race gametype as far as I can tell. This differs from the behavior in teerace, which is probably the only other still running race modification. Is this intentional? If so, why do you want to change that?

"only reload servers when changing tab" is unrelated to the rest of the changes. Could it be considered wanted behavior if the user can click the tab again to force a reload?

@timakro timakro changed the title added sv_race_server for race maps added sv_race_server for race maps & @timgame only reload servers when changing tab Oct 6, 2015
@timakro

timakro commented Oct 6, 2015

Copy link
Copy Markdown
Member Author

I added the other settings because I thought it could be useful for players who want to tune the settings for their raceservers.

The Race gametype doesn't kill the projectile on player kill, I recreated this for me it also seems like teerace hosts Race servers.

"only reload server when changing tab" changed only the behaviour of the ingame browser since this was change for the menu browser some time ago. Use the refresh button to reload servers. I also removed a ton of useless variables there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here.

@heinrich5991

Copy link
Copy Markdown
Member

I added the other settings because I thought it could be useful for players who want to tune the settings for their raceservers.

I think that all configuration options we add should have a clear use case, and should otherwise be avoided if possible. Adding configuration options has two downsides:

  1. We're stuck with them forever. Servers or maps might start relying on these.
  2. We're providing a less consistent playing experience.

The Race gametype doesn't kill the projectile on player kill, I recreated this for me it also seems like teerace hosts Race servers.

teerace hosts teerace server, which have the gametype "Race". As far as I know the behavior of them is to remove projectiles on player death.

Only reloading on tab switch sounds fine if that's the behaviour elsewhere as well. In future, you should try to split your pull request so each of them only implements one feature.

@timakro

timakro commented Oct 6, 2015

Copy link
Copy Markdown
Member Author

on teerace the grenade isnt killed, i tested this.

@heinrich5991 heinrich5991 self-assigned this Oct 6, 2015
@heinrich5991

Copy link
Copy Markdown
Member

It would be easier to review this if the pull request was split into the serverbrowser and sv_race_server part. If you want help with that, I can assist you.

As the interaction between sv_solo_server and sv_race_server isn't entirely clear, perhaps one could merge these variables by making sv_solo_server 2 the race server setting.

It seems the previous code for tried to not reload the server browser in case the user just switched to settings and back, your version does not preserve that, resulting in more refreshs.

I'm still trying to understand the implications of the changes in the laser code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment and the one below should probably be restored.

@timakro

timakro commented Oct 8, 2015

Copy link
Copy Markdown
Member Author

I'm going to open two seperate pull requests.

@timakro timakro closed this Oct 8, 2015
@timakro timakro changed the title added sv_race_server for race maps & @timgame only reload servers when changing tab added sv_race_server for race maps & only reload servers when changing tab Oct 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants