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

add "numPlayers" as condition in maprotation #1147

Merged
merged 4 commits into from Jan 5, 2020

Conversation

@Dimitriio
Copy link
Contributor

Dimitriio commented Dec 31, 2019

Hi,

Issue #1146 :
I just changed from numConnectedClients to numPlayingPlayers when the condition is verified.

I am new on the project so I didn't really know if I could test it easily in dev mode.
(I knew it from when I was playing Tremulous times ago).

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

Hi, welcome! :-)

You can do: daemonded -set vm.sgame.type 3 to load a shared lib (.so/.dll) sgame.

You can then set this in config/server.cfg:

set g_mapConfigs "map"

and this in config/map/default.cfg:

bot fill 3 humans
bot fill 3 aliens

To load bots at map startup.

See my work-in-progress scripts branch for more config samples.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

Also, maybe it would makes sense to rename numClients to numPlayers or something like that. Because the bug is not that the code does not do what it tells to do, but that it does something that seems useless.

@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Dec 31, 2019

@illwieckz do you prefer I totally remove the numClients condition as its not really useful or I keep it to not make a breaking change in the configuration of the maprotation ?

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

Well, a safe choice may be to add numPlayers condition alongside existing numClients.
There may be uncommon but useful need for counting bots as players (numClients), I think about some training server we had that relied heavily on bots.

@slipher

This comment has been minimized.

Copy link
Contributor

slipher commented Dec 31, 2019

numPlayingPlayers is not really what you want, since it does not count players on the spectator team. The CalculateRanks function has some good information. Maybe you could try subtracting the values of level.team[*].numPlayers instead.

@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Dec 31, 2019

@illwieckz I couldnt test my changes. I tried use the config you told me but I dont really know where to put them. I tried to put it where HOMEPATH is but when I launch the server, it stops with Idle server with no map — triggering watchdog.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

You have to load a map, or load a config that loads a map:

./daemon -set vm.sgame.type 3 -exec server.cfg
@slipher

This comment has been minimized.

Copy link
Contributor

slipher commented Dec 31, 2019

You have to load a map, or load a config that loads a map:

./daemon -set vm.sgame.type 3 -exec server.cfg

Also make sure to build the sgame-native-dll target each time you change the code, else the changes won't be tested.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

Yep, make sure you pass -DBUILD_GAME_NATIVE_DLL=ON to first cmake call.

@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Dec 31, 2019

I have took the files from your repo and I try ./build/daemonded -set vm.sgame.type 3 +exec <path to server.cfg> and now i have couldn't exec '<path to server.cfg' >

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

Paths are:

HOMEDIR/config/server.cfg
HOMEDIR/config/map/default.cfg
HOMEDIR/game/maprotation.cfg

I'm not sure about where the path to Unvanquished HOMEDIR is on Windows, it is probably %HOMEPATH%\Documents\My Games\Unvanquished

@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Dec 31, 2019

I am on Debian system so I don't know neithe hehe.
The root path I use is /home/dimitri/.local/share/unvanquished

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

so paths are:

/home/dimitri/.local/share/unvanquished/config/server.cfg
/home/dimitri/.local/share/unvanquished/config/map/default.cfg
/home/dimitri/.local/share/unvanquished/game/maprotation.cfg
@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Dec 31, 2019

Alright !!! Seems like it's good now !!!
Thanks for the help ! I was near the solution but a bit lost too. Should be okay now.

And this option have to be passed everytime i rebuild ? or cmake --build build -- -j4 is enough ?

Yep, make sure you pass -DBUILD_GAME_NATIVE_DLL=ON to first cmake call.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Dec 31, 2019

You do once:

cmake -H. -Bbuild -DBUILD_GAME_NATIVE_DLL=ON

then, as usual, you do any time you want:

cmake --build build -- -j4
@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Dec 31, 2019

In src/sgame/sg_main.cpp there is this part in the CalculateRanks method:

        // TODO: Use TEAM_ALL or the latter version for this everywhere
	level.team[ TEAM_NONE ].numPlayers += level.numPlayingPlayers;

that make my changes not working when there were players in the Human/Alien team as they are count in both spec & the team.

I can try do it if while I am on it. Or change my calculation rule but will need to be changed again when the quoted TODO will be done.

@Gireen

This comment has been minimized.

Copy link
Member

Gireen commented Jan 1, 2020

you could add a level.numConnectedPlayers and let it only count humans in CalculateRanks
which could also be useful in other places

src/sgame/sg_struct.h Outdated Show resolved Hide resolved
@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Jan 1, 2020

That looks great, can you edit the PR title? :-)

@Dimitriio Dimitriio changed the title MapRotation : counting realPlayer in the numClients condition add "numPlayers" as condition in maprotation Jan 1, 2020
@Dimitriio

This comment has been minimized.

Copy link
Contributor Author

Dimitriio commented Jan 1, 2020

I change the title to something more explicit. Let me know if it does not suffice.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Jan 1, 2020

that's good!

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Jan 1, 2020

my server is running both this change and the one from #1145 and it works as expected:

numPlayers map rotation condition

numPlayers map rotation condition

numPlayers map rotation condition

good job guys!

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Jan 4, 2020

@Dimitriio, can you rebase on master and squash those two commits as one (you only need to keep the second message):

  • MapRotation : counting realPlayer in the numClients condition
  • MapRotation : Add 'numPlayers' condition
@Dimitriio Dimitriio force-pushed the Dimitriio:master branch from fb788de to eab61cc Jan 5, 2020
@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Jan 5, 2020

Thank you for making Unvanquished great again :-)

@illwieckz illwieckz merged commit 5198bac into Unvanquished:master Jan 5, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.