gal_startvote (issue #65)

[spark512]

Changed gal_startvote behavior to follow the cvar gal_endonround_rtv. This way i achieved the desired effect based on my configurations, but you should check to see if this change breaks any desired effects for other configurations..

[evandrocoan]

Thanks. But on the original brad the default behavior is to change the map immediately, unless the argument -nochange is provided.

1. https://forums.alliedmods.net/showthread.php?t=77391#commands

I am going to add a new argument -roundend which does what your pull request is doing. So you need to call it as gal_startvote -roundend.

[spark512]

In brad's plugin the -nochange argument only changed the nextmap value, and left it as it is. The default gal_startvote command followed the settings within galileo, so if galileo was set to change the map after the round ended(which was our settings), then doing the gal_startvote command would also follow this behavior.

We've been using gal_startvote for years, i even still have the old configurations saved...

[evandrocoan]

if galileo was set to change the map after the round ended(which was our settings), then doing the gal_startvote command would also follow this behavior.

Sorry, I never noticed this behavior.

I reimplemented/rewrote most things based on the documentation provided by Brad, so it seems this was missed.

I am going to add check for the cvar gal_endonround when the gal_startvote command is issued:

// Indicates when a map should end when time runs out.
//
// 0 - end immediately when time runs out
// 1 - when time runs out, end at the current round end
// 2 - when time runs out, end at the next round end
//
// Default: 0
gal_endonround 1

[spark512]

Awesome, thanks.

[evandrocoan]

Actually, the cvar gal_endonround as written there, it is stating:

Indicates when a map should end when time runs out.

It is not saying anything about when a voting is started by the command gal_startvote. Them if the original galileo was behaving like that, it is bug.

Therefore I can only add a new cvar to control this behavior, because everybody who configured the plugin until today would have their configuration wrong, as on the Brad's version first page says for the command gal_startvote:

the map will be changed once the next map has been determined

[spark512]

Look.

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L94

g_handleMapChange is the bool which says galileo will handle the map changing. If you use the -nochange parameter, it sets it to false, and lets server automatically change map when time runs out, or another mapchooser to handle it.

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L594

Now, since galileo is handling the map change, and the vote is forced by gal_startvote, this map_manageEnd gets called.

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L2153-L2165

And in map_manageEnd galileo follows the endOnRound cvar and prevents map change until the round is finished.

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L809-L826

[evandrocoan]

Thanks for explaining. The code is clearly doing that. However its documentation on the first post and the on the settings file, does not. Therefore it is considered a bug on the code, when there is a different behavior, which can be considered fixed on this version.

If someone want to keep the original behavior, just need to the set cvar gal_general_options I added on the last commit to 2, on the new settings file. To help I can send the default settings set to 2. Moreover, new user which just migrate form the original Brads version, can keep the same/original settings file and the original behavior at the same time.

After some new tests I am going to release this new version.

[spark512]

the map will be changed once the next map has been determined

This can be wrongly interpreted. This line should've been written as:

the map will be changed by galileo once the next map has been determined

by galileo - meaning it will change the map based on the configurations defined in galileo.cfg

Documentation can be wrong. Documentation can be badly written, outdated or incomplete. You shouldn't rely on documentation, you should look at the actual source code to see what's going on. You want both plugins to give the same output, and you can achieve this by looking at the source code.

It's best to make both plugins behave the same way by default, and then have the option to enable your changes and fixes as extra settings and cvars.

I am actually trying to migrate from brad's plugin to yours, but these inconsistencies are preventing me from fully migrating on all servers. When i tried to migrate the config from brad's plugin to your version, it did not produce the same behavior. This is what brought me here to github..

Your changes and improvements are great, but dont force them on the user. Make your plugin exactly the same as brad's by default, and then allow cvars to enable your improvements.

[evandrocoan]

you should look at the actual source code to see what's going on

It is the opposite. The end user must not look into the source code to know what is going on.

This behavior was a bug on Brad's version. On the fix I did yesterday I put a new cvar to control it. So you can migrate your cfg and keep the behavior.

but these inconsistencies are preventing me from fully migrating

What more inconsistencies there are?

but dont force them on the user

I am not forcing anything. This was a bug on the original version. I probably doubled the existent cvars. You can customize much more things now.

Make your plugin exactly the same as brad's by default

I will not do exactly as Brad. I am going to fix all its bugs.

Documentation can be wrong

Documentation can have bugs also. On this case I assume the code is wrong, because that is what the end user sees and say while configuration the plugin.

by galileo - meaning it will change the map based on the configurations defined in galileo.cfg

There is no cvar on the galileo.cfg saying to control this behavior. The most close is the one used for the timelimit expiration, i.e., explicitly saying when time runs out:

// Indicates when a map should end when time runs out.
//
// 0 - end immediately when time runs out
// 1 - when time runs out, end at the current round end
// 2 - when time runs out, end at the next round end
//
// Default: 0
gal_endonround 1

By coincidence, Brad's version seems to be using this to control the gal_startvote behavior.

[spark512]

It's not coincidence, this is how it's coded, to follow the settings. gal_startvote is categorized as a forced vote, and forced votes are handled within map_manageEnd.

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L598

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L1301

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L1335-L1338

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L2155-L2159

This check for gal_endonround is for all vote scenarios, it doesnt make sense to have different behaviors for different vote types. If brad wanted it to be different he would've made another cvar like gal_rtvendonround or gal_startvoteendonround, etc...

https://gist.github.com/spark512/d5cbb3a21fb0c391a3cf454ff54f4ccd#file-galileo-sma-L809-L826

It is the opposite. The end user must not look into the source code to know what is going on.

Documentation can have bugs also. On this case I assume the code is wrong, because that is what the end user sees and say while configuration the plugin.

Yes, end users dont care about source code, but it's the coder who needs to check it to make sure the plugin's code and behavior is consistent.

Why do you assume the code is wrong?

I am saying you've misinterpreted the documentation of gal_startvote

the map will be changed once the next map has been determined

It doesnt say map will be changed right after map has been determined, it says once map has been determined. Do we know if this is right away? End user doesnt know, he needs to check how he has configured gal_endonround to know how gal_startvote will behave. It's global for all votes.

[evandrocoan]

it's the coder who needs to check it to make sure the plugin's code and behavior is consistent. It doesnt say map will be changed right after map has been determined

You are correct. It does not specify when it will change. So it is considered an undefined behavior, therefore anything may happen and there is nothing which can predict this behavior.

Why do you assume the code is wrong?

As it is, it is indeed a bug on the documentation. I am going to fix it. However as it is undefined behavior, the code can do whatever it want to, so there is no wrong behavior. After the documentation update, the code and the documentation are going to be synced accordingly without any undefined behavior. At least on this part of the code. Someone else may find another inconsistencies with the documentation and code in the future.

This check for gal_endonround is for all vote scenarios, it doesnt make sense to have different behaviors for different vote types.

If it makes sense or not, it is not up to only you and me determine. You need to do a extensive research and interview with different people who uses the plugin to determine whether they would like this option or not. In my option it is perfectly valid to have both settings, where actually this is the setting I am using on my server. When I do a voting by gal_startvote I expect by default the map to change immediately, but when the time runs out on the last round I expect the plugin to wait the round to finish.

If brad wanted it to be different he would've made another cvar like gal_rtvendonround or gal_startvoteendonround, etc...

If he wanted different behavior, he should not write right up on the documentation this:

// Indicates when a map should end when time runs out.
...
gal_endonround 1

It should be written instead:

// Indicates when a map should end after a forced voting has finished or when the time runs out.
...
gal_endonround 1

If this just about, were there on the documentation, then his code would be correct and consistent.

he needs to check how he has configured gal_endonround to know how gal_startvote will behave. It's global for all votes

The user has no indication at all from the documentation of gal_endonround that it controls the behavior of the gal_startvote. The user would have to guess, find out by accident while testing or look into the code by himself.

// Indicates when a map should end when time runs out.
...
gal_endonround 1