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

Chunk system #29

Merged
merged 21 commits into from
Aug 26, 2023
Merged

Chunk system #29

merged 21 commits into from
Aug 26, 2023

Conversation

titusio
Copy link
Contributor

@titusio titusio commented Aug 18, 2023

this is a first version of a chunk system mentioned in #16.
There are some areas which need some work.

  • As mentioned in the issue, some generators won't be able to use this system, so introducing a base class for generators using the chunk system could be helpful for differentiating those 2 kinds of generators.
  • I think there has to be a solution for saving the chunks. There could be a signal with the grid as a parameter which allows the developer to save the data. A method for adding the data back in could be helpful too.
  • In the test scene I added a simple script to load the chunks. Adding a script loading/unloading chunks around a referenced Node2D shouldn't be too hard to add.
  • In the test scene I created a unique copy of the Terraria-like settings and disabled the random seed generation. This setting caused problems because the noise on the Caver modifier was different on each chunk. I don't have a solution for this problem.
  • This PR currently lacks an unload function.
  • Currently only the HeightMapGenerator is implemented.
  • There is not documentation for the chunk system.

I hope this PR matches the vision for this plugin :) Any kind of help would be really appreciated ^^

@BenjaTK BenjaTK added the ✨ enhancement New feature or request label Aug 18, 2023
@BenjaTK BenjaTK assigned BenjaTK and unassigned BenjaTK Aug 18, 2023
@BenjaTK BenjaTK self-requested a review August 18, 2023 18:26
@titusio
Copy link
Contributor Author

titusio commented Aug 20, 2023

I removed some unnecessary code duplication. If we'd introduce a base class for all modifiers which can work with chunk, we could erase this.

@BenjaTK
Copy link
Owner

BenjaTK commented Aug 20, 2023

Just tried it in my editor, looks great! I'm debating rn between having the chunk loader in its own node like it is at the moment, or in the Generators. I think I like the idea of having a ChunkLoader node where you set a radius, an (optional) actor, and maybe an update_rate so the chunk loader will automatically load chunks around the actor. What do you think? I don't know if that's the idea you had.

@BenjaTK BenjaTK added this to the v1.0.0 milestone Aug 20, 2023
@BenjaTK
Copy link
Owner

BenjaTK commented Aug 20, 2023

Btw, could we add a has_chunk function? Could be nice for the chunk loader to avoid regenerating chunks that were already generated.

@BenjaTK
Copy link
Owner

BenjaTK commented Aug 20, 2023

And about the random noise seed problem, maybe we only change the seed in the apply function? Instead of the apply_area one which is called by both the normal and chunked ones. That should work? Idk how to fix the one for the generator itself, but maybe that's just something the user has to set at the start.

@titusio
Copy link
Contributor Author

titusio commented Aug 20, 2023

I think a ChunkLoader node would be best because it makes it clear, that this system is optional. I will add the missing has_chunk method together with a better ChunkLoader.
Since the apply_chunk method has access to the generator object, I could imagine using a random seed generated by the generator itself. What do you think about introducing a ChunkAwareModifier resource (I am really not sure about the name). This way this functionality could be written once and the modifiers would only have to overwrite the apply_area method.

@BenjaTK
Copy link
Owner

BenjaTK commented Aug 20, 2023

What do you think about introducing a ChunkAwareModifier resource (I am really not sure about the name). This way this functionality could be written once and the modifiers would only have to overwrite the apply_area method.

That sounds good!

@titusio titusio marked this pull request as ready for review August 25, 2023 13:32
@titusio
Copy link
Contributor Author

titusio commented Aug 25, 2023

I think this PR is ready now. Enabling the user to save the data is beyond the scope of this but definitely worth investing some time in. Other than that I think this is finished

@titusio titusio changed the title [WIP] Chunk system Chunk system Aug 25, 2023
addons/gaea/generators/generator.gd Outdated Show resolved Hide resolved
addons/gaea/generators/generator.gd Outdated Show resolved Hide resolved
addons/gaea/modifiers/carver.gd Show resolved Hide resolved
addons/gaea/modifiers/noise_painter.gd Show resolved Hide resolved
addons/gaea/chunk_loader.gd Outdated Show resolved Hide resolved
@BenjaTK
Copy link
Owner

BenjaTK commented Aug 26, 2023

LGTM!

@BenjaTK BenjaTK merged commit 40e1169 into BenjaTK:main Aug 26, 2023
This was referenced Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants