-
Notifications
You must be signed in to change notification settings - Fork 41
Move biome definitions to YAML configuration with hot reload support #404
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
Move biome definitions to YAML configuration with hot reload support #404
Conversation
|
Anything here I need to fix or change? |
Volte6
left a comment
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.
Just a couple small changes requested. The toughest one beign the fileloader change.
Hit me up if it's confusing or I can help with that.
…ically - Enabling biome definitions outsite of code - Biomes reload in the same way as items
"reload biomes" will now hot reload changes, like on items.
1. YAML tags changed to lowercase - Changed yaml:"darkArea" to yaml:"darkarea" - Changed yaml:"litArea" to yaml:"litarea" - Changed yaml:"requiredItemId" to yaml:"requireditemid" - Changed yaml:"usesItem" to yaml:"usesitem" - All YAML tags now follow the lowercase convention as requested 2. Implemented Loadable interface methods - Added Id() method that returns the biome ID as a string - Added Validate() method with proper validation logic - Added Filepath() method that returns the individual file path - BiomeInfo now satisfies the Loadable[string] interface 3. Refactored to use fileloader.LoadAllFlatFiles pattern - Converted from single biomes.yaml file to individual biome files (e.g., city.yaml, forest.yaml) - Changed loading mechanism to use fileloader.LoadAllFlatFiles[string, *BiomeInfo]() - Created individual biome files in _datafiles/world/default/biomes/ directory I looked at races/mobs/items and tried to follow the same pattern. Additional fixes and changes: - Updated all references to BiomeInfo fields to use the new exported fields - Fixed method calls from Symbol() to GetSymbol() and Name() to Name field access - Fixed the return type of GetBiome() to return *BiomeInfo instead of BiomeInfo Also had to fix an unreachable code issue in mutators.go (This was a compiler error due to unreachable code after line 49 because of a 'return false') This may be intentional, but the compiler will not make the binary without this change.
Added missing hard coded biomes as regular biome files: - Land - Dungeon Also added them back as hard coded fallbacks in biomes.go
Volte6
left a comment
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.
Note: The _datafiles/world/default files need to be copied to _datafiles/world/empty as well... for now. We will phase that out soon, but for now we need to keep them in parity.
Volte6
left a comment
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.
Overall, looking very good. A few small changes and questions left but fairly minor.
I think we can be a bit more cavalier about backwards compatibility in cases like this - we don't want to create a habit of having to duplicate data in code AND flatfiles as we improve things.
- Removed all hardcoded biomes from biomes.go - Replaced with single default fallback biome when no files are loaded - Removed ValidateBiomes() function and its call from main.go - Changed GetAllBiomes() to return []BiomeInfo instead of []*BiomeInfo to prevent accidental modifications - Reverted mutators.go changes that were accidentally included in this branch
…idual biomes to /empty
- Added defensive nil checks in mapper.go before calling biome methods - Ensured default biome always exists after LoadBiomeDataFiles() is called - Added warning log when a requested biome is not found, showing biome name, room ID, and zone - Improved biome fallback logic to prevent nil pointer dereferences - Added mudlog import to rooms.go for biome warning messages
- Added validateRoomBiomes() function in mapper that checks all rooms during PreCacheMaps() - Logs warnings at startup for rooms without biomes (when zone also has no default) - Logs warnings for rooms referencing non-existent biomes - Provides summary count of missing and invalid biome references
|
And btw. thanks for the really great review notes. I am learning a ton from this :) |
main.go
Outdated
|
|
||
| // Load biomes before rooms since rooms reference biomes | ||
| rooms.LoadBiomeDataFiles() | ||
|
|
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.
Nickpick - remove empty line. Non blocking.
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.
Done, fixed :)
Volte6
left a comment
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.
One small comment, not a big deal. Code looks good.
Summery
This PR refactors the biome system to move biome definitions from hardcoded values to external YAML configuration files, enabling dynamic loading and hot reloading of biome data.
Key Changes
Technical Details
Benefits