-
Notifications
You must be signed in to change notification settings - Fork 171
Split structures out of biomes #2131
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
base: master
Are you sure you want to change the base?
Conversation
|
does this allows addons to add structures to preexisting biomes? 🥺 |
With #2129 yes. Not by itself though. |
src/utils/hash.zig
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this can stay in utils.zig, it doesn't need its own file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I have a vague memory of previous attempt to make hashGeneric widely available and I think it was rejected by Quantum with argument that generic functions like this one tend to cause problems and hashGeneric should remain isolated from the rest of the codebase as part of world gen code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. We could maybe stick it in terrain.zig instead? At least then it'd be accessible to more terrain-related structs. Currently only Biome and StructureTable need it due to both using SimpleStructureModel.
I could also see the argument for avoiding this PR altogether and just throwing everything into biomes.zig. If that's ultimately what's wanted I don't mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put the hashing functions in terrain.zig instead for now.
Also rebased onto QD's latest commit since there were changes there in conflict with mine.
9a7ed01 to
cf561be
Compare
| const terrain = main.server.terrain; | ||
| const Biome = main.server.terrain.biomes; | ||
|
|
||
| pub const SimpleStructureModel = struct { // MARK: SimpleStructureModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update this to the state after #2125
|
|
||
| pub const structure_building_blocks = @import("structure_building_blocks.zig"); | ||
|
|
||
| pub fn hashGeneric(input: anytype) u64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only meant to be used by the biomes, so pelase keep it in biomes.zig for now.
This PR splits out structures and hashing from biomes.zig into structures.zig and src/utils.zig respectively.
This change should be done for the following reasons: