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

On-demand background navmesh generation #2353

Merged
merged 3 commits into from Jan 17, 2023
Merged

Conversation

slipher
Copy link
Contributor

@slipher slipher commented Jan 3, 2023

Stacked on #2352.

On-demand navmesh generation in the background in sgame is ready to test out. Just load a map and add a bot. Well also you need valid navmeshes to not already exist; you can delete them from <homepath>/game/maps/ if applicable, or edit NAVMESHSET_VERSION in the source to invalidate all existing meshes.

@illwieckz
Copy link
Member

Impressive!

]/bot add * aliens 
==== Bot Navigation Initialization ==== 
[bot] Bot#1 is connecting… 
[bot] Bot#1 entered the game 
Generating navmesh for level0 
Generating bot navigation mesh for level0... 0% 
Generating navmesh for level1 
Generating navmesh for level2 
Generating navmesh for level2upg 
Generating navmesh for level3 
Generating navmesh for level3upg 
Generating navmesh for level4 
Generating navmesh for human_naked 
Generating navmesh for human_bsuit 
==== Bot Navigation Initialization ==== 
 loading navigation mesh file 'maps/rsmse-level0.navMesh'... 
 loading navigation mesh file 'maps/rsmse-level1.navMesh'... 
 loading navigation mesh file 'maps/rsmse-level2.navMesh'... 
 loading navigation mesh file 'maps/rsmse-level2upg.navMesh'... 
 loading navigation mesh file 'maps/rsmse-level3.navMesh'... 
 loading navigation mesh file 'maps/rsmse-level3upg.navMesh'... 
 loading navigation mesh file 'maps/rsmse-level4.navMesh'... 
 loading navigation mesh file 'maps/rsmse-human_naked.navMesh'... 
 loading navigation mesh file 'maps/rsmse-human_bsuit.navMesh'... 
]/bot add * humans 
[bot] Bot#2 is connecting… 
[bot] Bot#2 entered the game 

Do we need the Generating bot navigation mesh for level0... 0% log line? It seems redundant.

@illwieckz
Copy link
Member

I have mixed feelings about “Put helpful info in bot pings” commit as some generic purpose or legacy tools (like XQF) detect bots by checking that player ping is zero.

@illwieckz
Copy link
Member

illwieckz commented Jan 3, 2023

For bot info, if we don't have any bot skill level above 9 we can reuse the B key (slotBots) and instead of writing a small b when this is a bot we would write the bot skill level as a digit or a letter for any other status. We may have to also edit the Mantis to keep the compatibility but no more (and the change in Mantis would be trivial).

@slipher
Copy link
Contributor Author

slipher commented Jan 3, 2023

Do we need the Generating bot navigation mesh for level0... 0% log line? It seems redundant.

This is a message sent to all clients, while the other ones are just normal logs in the server console.

I have mixed feelings about “Put helpful info in bot pings” commit as some generic purpose or legacy tools (like XQF) detect bots by checking that player ping is zero.

That wasn't working anyway as the bot pings were previously random numbers from 50 to 99. I have vague recollections that a 0 ping supposedly triggers bugs...

@illwieckz
Copy link
Member

I have mixed feelings about “Put helpful info in bot pings” commit as some generic purpose or legacy tools (like XQF) detect bots by checking that player ping is zero.

That wasn't working anyway as the bot pings were previously random numbers from 50 to 99. I have vague recollections that a 0 ping supposedly triggers bugs...

Ah… but what do you think about slotBots? We already have a field to store one alphanumeric (or any other 1-byte-sized char) per bot.

@DolceTriade
Copy link
Member

Nice will test later today!

@slipher
Copy link
Contributor Author

slipher commented Jan 3, 2023

Ah… but what do you think about slotBots? We already have a field to store one alphanumeric (or any other 1-byte-sized char) per bot.

The ping is also a required field for every bot, and one that is already displayed to users. So to me it's nice to put something useful there without writing any new code to process it.

@illwieckz
Copy link
Member

src/sgame/sg_bot.cpp Outdated Show resolved Hide resolved
If the cvar g_bot_nav_onDemand is enabled, then the sgame will
generate missing navmeshes when a bot is added. This will block the
server main thread for however long it takes.
@illwieckz
Copy link
Member

I tested it with all official maps and it works.

LGTM if Put helpful info in bot pings commit is moved as a separate PR so we can merge the navgen feature while we discuss about that bot ping thing in a less priory way.

If g_bot_navgen_onDemand is 1 (the default), then the navmesh will
start being generated in the background (a little bit each frame).
Bots can join a team immediately, but do not spawn until the generation
has completed.

g_bot_navgen_msecPerFrame controls how much generation is done per
frame, trading off faster generation vs. laggy server.
@slipher slipher marked this pull request as ready for review January 16, 2023 11:42
@slipher
Copy link
Contributor Author

slipher commented Jan 16, 2023

Removed the helpful bot pings commit and add the prefix Server: to the broadcast message Server: generating bot navigation mesh for $1$... $2$%.

@illwieckz
Copy link
Member

illwieckz commented Jan 16, 2023

One minor issue I noticed but I don't consider this blocking the merge, is that doing navedit enable before there is any navmesh will forbid to generate navmesh without reloading the map first:

]/devmap forlorn
]/navedit enable level0
==== Bot Navigation Initialization ==== 
Debug: Failed to open 'game/maps/forlorn-level0.navMesh' for reading: No such file or directory 
Warn: Cannot load navigation mesh file for level0 
Warn: Couldn't load navmeshes 

]/bot fill 3 
==== Bot Navigation Initialization ==== 
Warn: Cannot load navigation mesh file for level0 
Warn: Couldn't load navmeshes 
]/bot fill 3 
Warn: Navmesh initialization previously failed, doing nothing 
Navigation mesh files unavailable for this map 

]/devmap forlorn
]/bot fill 3 
==== Bot Navigation Initialization ==== 
Generating navmesh for level0 
[bot] Bot#1 is connecting… 
[bot] Bot#1 entered the game 
[bot] Bot#2 is connecting… 
[bot] Bot#2 entered the game 
[bot] Bot#3 is connecting… 
[bot] Bot#3 entered the game 
[bot] Bot#4 is connecting… 
[bot] Bot#4 entered the game 
[bot] Bot#5 is connecting… 
[bot] Bot#5 entered the game 
[bot] Bot#6 is connecting… 
[bot] Bot#6 entered the game 
Generating bot navigation mesh for level0... 0% 
Generating navmesh for level1 
Generating navmesh for level2 
Generating navmesh for level2upg 
Generating navmesh for level3 
Generating navmesh for level3upg 
Generating navmesh for level4 
Generating navmesh for human_naked 
Generating navmesh for human_bsuit 

Copy link
Member

@illwieckz illwieckz left a comment

Choose a reason for hiding this comment

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

LGTM.

@illwieckz illwieckz added this to the Beta 54 (v0.54.0) milestone Jan 16, 2023
@slipher
Copy link
Contributor Author

slipher commented Jan 16, 2023

One minor issue I noticed but I don't consider this blocking the merge, is that doing navedit enable before there is any navmesh will forbid to generate navmesh without reloading the map first:

Yeah that was by design. Although I guess we could make /navedit ignore the failure init status and retry since it is in response to (presumably) a human input. The main thing is not to reattempt initialization on every bot command since there may be several of them in a script.

Technically you could still generate navmeshes with /navgen after a failure, but what you can't do is load them.

@slipher slipher merged commit 56aa86a into Unvanquished:master Jan 17, 2023
6 checks passed
@slipher slipher deleted the nav-wip branch January 17, 2023 07:34
@necessarily-equal
Copy link
Contributor

There is this warning:

src/sgame/sg_bot_nav.cpp:180:24: warning: enumeration value ‘GENERATING’ not handled in switch [-Wswitch]
  180 |                 switch ( G_BotSetupNav( &bot, &model->navHandle ) )
      |                        ^

I think it would be good to handle it, but I'm not sure how. Maybe case navMeshStatus_t::GENERATING: return; to avoid requesting things twice?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants