Open GooberRF opened 2 months ago
This implementation makes no sense.
vote random
should not change current_level_index
.Further I believe filter fragments would be useful. For example: vote random:blunder
.
The map rotation should be shuffled upon startup.
The current implementation results in a random map being picked every time the level changes. Shuffling the map rotation on startup would result in a static list as normal (just one that's in random order). With your suggested approach, after the rotation is complete, it will repeat in the same identical order each time.
That said, while I generally prefer the current implementation personally, I see value in your suggested approach as well, it's just different - your approach would mean maps are not repeated until the entire list is repeated. I think the current implementation would usually be preferred for servers with few maps in rotation, and your suggestion would usually be preferred for servers with lots of maps in rotation.
Perhaps there is room for both approaches:
$DF Random Level Order: true
+No Repeats Until All Played: true
Something like that, perhaps, to offer server operators both options, since there are use cases for each.
vote random should not change current_level_index.
Why not? It's an elegant solution, and after a random next map has been voted for, the current level index isn't needed anymore. Also, unless I'm mistaken, this is effectively how vote prev
works currently.
I believe filter fragments would be useful. For example: vote random:blunder.
I doubt it would get much use if I'm honest, but I do also think this would be neat - as a future addition. In the interest of avoiding scope creep, I'm considering it out of scope for this PR.
I added a "shuffle_maps" command to shuffle the maplist based on the feedback above. Considered having a dedicated server config item to do this at launch as well, but that feels overly confusing and unnecessary. This method is much more intuitive and flexible, plus server operators can emulate the functionality of if it were in the dedicated server config file by just putting "shuffle_maps" in gamestart.txt.
I added a "shuffle_maps" command to shuffle the maplist based on the feedback above.
Your shuffle implementation is wrong. It is going to duplicate and lose maps.
With your suggested method, after the rotation is complete, it will repeat in the same identical order each time.
You can shuffle again after a complete map rotation if you want to.
That said, while I generally prefer the current implementation personally, I see value in your suggested method as well, it's just different - your method would mean maps are not repeated until the entire list is repeated. I think the current implementation would usually be preferred for servers with few maps in rotation, and your suggestion would usually be preferred for servers with lots of maps in rotation.
If a map rotation has 3 maps your implementation has a ⅔ chance it is going to select whatever was first or second place for third place. This behavior is neither intuitive nor desirable. It literally makes no sense.
Why not? It's an elegant solution, and after a random next map has been voted for, the current level index isn't needed anymore.
You are going to lose your place in your map rotation.
Your shuffle implementation is wrong. It is going to duplicate and lose maps.
I'll have a look into that - to my understanding that would not happen, but I'm admittedly not very experienced with this stuff, so it's completely possible I messed it up. Will look further into it, thanks!
If a map rotation has 3 maps your implementation has a ⅔ chance it is going to select whatever was first or second place for third place. This behavior is neither intuitive nor desirable. It literally makes no sense.
The behaviour is intended to effectively "roll the dice" and select a random map, excluding only the one you currently have selected. To my mind, the circumstance you described is it behaving by design... though, it would probably be a little silly to try a "randomized rotation" with only 3 levels anyway.
You are going to lose your place in your map rotation.
This is not an issue with the current behaviour. By design, you have no "place in the map rotation" - it treats the map "rotation" as a pool of available maps to pick a random entry from when needed. I gather that you disagree with the rationale behind the way I've implemented the feature, but with the feature working as it's intended, losing your place in the map rotation is not a concern.
EDIT: And actually, I'd go a step further when it comes to voting and the map_rand command - if you use one of those on a server that does not have $DF Random Level Order turned on, "losing your place in the map rotation" would be the expected behaviour. If you are currently on map index 5 and you enter map_rand and get randomly swapped to map index 2, you naturally would expect the level loaded after that to be map index 3, not map index 6.
I'll have a look into that - to my understanding that would not happen, but I'm admittedly not very experienced with this stuff, so it's completely possible I messed it up.
I looked again. I misread your line! It is not wrong. May as well use modern C++ with std::shuffle
and something like std::mt19937
too.
To my mind, the circumstance you described is it behaving by design... though, it would probably be a little silly to try a "randomized rotation" with only 3 levels anyway.
My point was it is not going to be uncommon for maps to repeat sometimes ridiculously so whereas my method does not.
If you are currently on map index 5 and you enter map_rand and get randomly swapped to map index 2, you naturally would expect the level loaded after that to be map index 3, not map index 6.
I was referring to this behavior. I think it is bizarre. vote dm02
does not destroy your position in the rotation. No one expects or wants vote random
to.
I looked again. I misread your line! It is not wrong.
I updated it to be formatted in a more readable manner as well.
I was referring to this behavior. I think it is bizarre. vote dm02 does not destroy your position in the rotation. No one expects or wants vote random to.
I believe I misunderstood you before, apologies! There's also no reason for vote rand or map_rand to need to do this, I have updated get_rand_level_filename
to avoid this behaviour.
I've overhauled the PR a bit, now it contains the following:
$DF Dynamic Rotation dedicated server config: if true, when the server reaches the last level in its rotation, shuffle the list of available levels
$DF Dynamic Rotation
should shuffle upon starting so a map rotation is different upon every run.
$DF Dynamic Rotation
should shuffle upon starting so a map rotation is different upon every run.
That was my original plan, but when I got to implementing I decided the current way offered more flexibility and therefore made more sense. As-is, the server operator gets to decide the first order, then future rotation orders are shuffled. If the server operator wants to they can always run the list of levels through random.org or something to shuffle even the first rotation (as they have been able to forever), this feature adds the part which up to this point was inaccessible: shuffling the list after the first rotation.
I'm not necessarily opposed to changing it so that it also shuffles the first list, but I did leave that part out intentionally in the current iteration, because I thought it offered more flexibility this way without taking away any capabilities.
It is essential so players do not have to play a stale first rotation. Server operators would not want to continually randomize their config manually.
A good example of shuffling after starting is a run map server. You put every map into dedicated_server.txt and append any which may be released. I would not want first rotation to be the same for every run. Further if you change things every now and then (even a PC reboot) you keep resetting back to the same rotation which can take a long time for run maps.
Voting a random map from a user_maps is useful too. My servers had a shuffled rotation so players never encountered the same rotation twice and random map voting using user_maps. I feel both options combined gave the most flexibility. But you could have both forms of random maps. Suffled rotations removes the need for random maps from the rotation so you would only need my form of random maps.
I've made some changes:
get_rand_level_filename
and shuffle_level_array
to use new RNG methodJust fixed a bug in the map_rand
and vote rand
logic. current_level_index
is not updated when the level is manually changed to a specific level (makes sense, to maintain the position in the rotation for when that map is done), but I had previously been using that as the check to verify map_rand
and vote rand
don't select the current level. It worked fine before when the level currently loaded is from the rotation, but not when the current level was manually loaded by filename (or a previous map_rand
).
New behaviour checks against the currently loaded level filename rather than index, so map_rand
and vote rand
should always work as expected now.
This PR adds 3 features:
Resolves #52