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

Error in noise_generator.gd when using ThreadedChunkLoader2D #126

Closed
Ronzan opened this issue Jun 18, 2024 · 5 comments
Closed

Error in noise_generator.gd when using ThreadedChunkLoader2D #126

Ronzan opened this issue Jun 18, 2024 · 5 comments

Comments

@Ronzan
Copy link

Ronzan commented Jun 18, 2024

Hello

Not sure if I'm doing something wrong, but I get following error:

E 0:00:00:0801   noise_generator.gd:72 @ generate_chunk(): Caller thread can't call this function in this node (/root/Main/NoiseGenerator). Use call_deferred() or call_thread_group() instead.
  <C++ Error>    Condition "!is_accessible_from_caller_thread()" is true. Returning: (ERR_INVALID_PARAMETER)
  <C++ Source>   scene/main/node.cpp:3630 @ emit_signalp()
  <Stack Trace>  noise_generator.gd:72 @ generate_chunk()
                 chunk_loader_2d.gd:86 @ _update_loading()
                 threaded_chunk_loader_2d.gd:26 @ <anonymous lambda>()

The setup is working, generating chunks as the player moves around - it lags a lot when chunks are generated though.
The setup looks like this:
image

I've tried to search a bit and found a few places that mentions signals not being thread-safe - but I really don't know since this is my first Godot test project :)
I can make a full repro if needed, just thought I would check here to see if I've done something obvious wrong.

/Ronzan

@cullumi
Copy link
Contributor

cullumi commented Jun 18, 2024

Hi there @Ronzan!

This is indeed because signals aren't thread-safe. Ideally there will be a second pass (by me, most likely) on a few of the generators to make sure such things are thread-safe out of the box.

For now, wrap those signals with a lambda, then call_deferred them, similar to in the Heightmap generators:

(func(): chunk_updated.emit(chunk_position)).call_deferred() # deferred for threadability
(func(): chunk_generation_finished.emit(chunk_position)).call_deferred() # deferred for threadability

It's not exactly clean looking, persay, but it is the best way to fix this issue based on what I've read in Godot's docs.

@Ronzan
Copy link
Author

Ronzan commented Jun 19, 2024

Hi @cullumi
That removed those errors thank you :)
Btw, do I need to configure or do anything else to get the whole threaded chunkloading/generation/rendering system running smooth? While it does feel a bit smoother than non-threaded, I still have noticeable lag spike when new chunks/tiles are generated. Feels like I'm missing something :)

@cullumi
Copy link
Contributor

cullumi commented Jun 19, 2024

Happy to help! :D

After doing some stress-testing, it seems that a bunch of deferred calls to TileMap.set_cell() and TileMap.erase_cell() in TilemapGaeaRenderer's _draw_area method were being batched together and run all at the same time.

The solution? Replace those deferred calls with thread_safe calls. This should help because now tile changes will be guaranteed to be paced out as they become ready; should smooth things out a bit.

In TilemapGaeaRenderer, try changing the following lines from:

tile_map.erase_cell.call_deferred( l, Vector2i(x, y))
(...)
tile_map.set_cell.call_deferred(
    tile_info.tilemap_layer, tile, tile_info.source_id,
    tile_info.atlas_coord, tile_info.alternative_tile
)

to:

tile_map.call_thread_safe("erase_cell", l, Vector2i(x, y))
(...)
tile_map.call_thread_safe("set_cell",
    tile_info.tilemap_layer, tile, tile_info.source_id,
    tile_info.atlas_coord, tile_info.alternative_tile
)

Hopefully that helps with the lag spikes. (I'll likely be making PR with these changes tomorrow if someone else doesn't beat me to the punch; it's late and I gotta work tomorrow morning lol)

@Ronzan
Copy link
Author

Ronzan commented Jun 19, 2024

Thanks again @cullumi
It did improve a bit. If I disable "Erase Empty Tiles" on the ThreadedTilemapGaeaRender node the spikes are gone in the y direction (moving up and down)! I still have noticeable spikes moving left and right...
I think I'm running all nodes with default values (except for the noise gen tiles data).
I'm not missing something basic, like setting he process thread group on the node, running in debug mode or something?
I am a complete Godot noob after all :)

Running the profiler, it looks like it is renderers and the chunkloader causing the spikes:
image

Should I make an isolated project and try and reproduce? or record a clip?
Thanks again for you work and assistance - much appreciated :)

@BenjaTK
Copy link
Owner

BenjaTK commented Jun 21, 2024

Fixed in #127

@BenjaTK BenjaTK closed this as completed Jun 21, 2024
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

No branches or pull requests

3 participants