-
-
Notifications
You must be signed in to change notification settings - Fork 154
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
Use separate resource files for each region #374
Conversation
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.
Thanks for working on this.
Here's what I'm thinking we'll do for upgrades:
- Terrain3DRegionManager:Object is the new version of Storage that you're working on.
- Terrain3DStorage:Resource will be a deprecated class with all of the bound functions.
- Storage calls will be redirected to RegionManager so user code won't change. But user's scene files will as the storage resource will stop being saved.
- A
Setup Wizard...
button above the storage directory will appear that will guide users to select their old data file, and the new directory, so we can split the files. - In a future version, once users no longer have Storage as a resource saved in their scenes, we will rename RegionManager:Object back to Storage:Object. Once again their code won't change.
So you could get started on a setup wizard.tscn and a button in the inspector. It should include:
- "This version of Terrain3D uses separate storage files; one for each region. Select your old storage file below, and the directory where you'd like save the new files so we can convert your data."
- "File names include the region coordinates for convenience. So an old storage file like:
world.res
can instead be changed toworld/terrain3d_01_03.res
,world/terrain3d_01_04.res
. Positive coordinates are represented by underscores (_), negatives by hyphens (-)." - Old Storage resource file: [xxxxx]
- New Storage Directory: [xxxxxx]
- Hide SetupWizard for this project (Project settings)
- Hide SetupWizard for all projects (Editor settings)
- Cancel, OK
Here's are some issues I found while testing:
-
Upon loading the node demo, I clicked the file directory, created a new blank folder, and accepted it. The existing regions did not clear. They are still there.
- Then I saved and Godot crashed. On the filesystem, one file is there terrain_-01-01.res. Which looks like the correct size.
-
I removed one of the four regions and saved. All 4 files show in the filesystem panel. However, when I restarted, the region is gone. I see the file is removed in windows. I think you need to unload/unref the resource so godot closes it.
-
I removed all regions and it did in the display, but it did not remove one file:
Terrain3DStorage#1768:remove_region: Failure to remove region file at res://terrain/terrain3d__00-01.res
- When I restarted, that one file loaded again.
-
I added a region, then removed it before saving and got this error:
Terrain3DStorage#1825:remove_region: Failure to remove region file at res://terrain/terrain3d__06_00.res
-
I dragged in the Player and World scenes and can get it to play with 49 regions. 50 and up starts to produce vulkan errors. These might be bugs out of scope for this PR. However it does load them all in the editor, so maybe not.
-
I tweaked the importer.gd script (adding:
var storage: Terrain3DStorage
) and upon importing it reportedERROR: Terrain3DStorage#7944:import_images: Storage not initialized
I haven't figured out how to solve this.
Vulkan is out of my wheelhouse. If it's an issue with me, I may not be able to fix it.
That makes sense; I think it's trying to instantiate a terrain storage that's designed to be a singleton, so what you're doing is actually incorrect. Variables should probably be defined as var storage:Terrain3DStorage = null. The real issue is that it should give you that message that you need to get the singleton through Terrain3D. I'm not sure how to do that. |
Are you using force rebuild or clear? Use debug logging and get the log when removing a region. Then look at the debug log using the main branch when removing a region. What's the difference? Notepad++ or diff can easily compare two files. You should have logging in all of your new functions.
How many regions can you run the game with on your computer? What about in the main branch? If your branch has no issue, they should be the same.
You're right, adding that one line didn't fix the import script. It also needs something like:
You should fix this script in this PR.
Instance, not singleton. Only Terrain3D can be used as a singleton. |
|
My computer can handle about 130 regions before crashing. |
The only outstanding things I can think of are:
|
Should clarify that I have now added the wizard. |
You've done |
When did I do that? I haven't purposefully done either. This branch hasn't interacted with main, or any other branch, at all since rebasing last week, or whenever that was. |
I have no recollection of doing this... nor do I have any idea how to undo it. |
e135e9b
to
8f2bbf4
Compare
@Xtarsia Fixed those issues. Use this PR for now. The other PR is for the conversion wizard, which I haven't started reviewing yet. Update Still need to test height ranges. |
Update_height functions seem to properly work on sculpting and undo. Terrain3DObjects also works. Not sure why it didn't work yesterday. So, I think this PR is done enough for wider testing in the main branch. Now I'll work on the conversion PR #476 before merging. |
c75e549
to
ad12d8a
Compare
… in Storage; Fix sanitize_map for 16-bit
Rebuild _region_locations when region_map dirty Generate colormap mipmaps for edited regions only on edit (regression Drop storage map api)
ad12d8a
to
82f3133
Compare
Update by TokisanGames
Separates the monolithic storage resource file into one file per region.
Fixes #356
Fixes #165
Fixes #159
Fixes #419
TODO:
Test:
See #433 for API changes
More API changes
Functions
get_region_location_from_id
get_region_locationi
get_region_id(position)
get_region_idp(position)
,get_region_id(location)
has_region(position)
has_regionp(position)
,has_region(location)
get_height_rid
get_height_maps_rid
get_control_rid
get_control_maps_rid
get_color_rid
get_color_maps_rid
set_height_maps
set_control_maps
set_color_maps
set_map_region
get_map_region
set_maps
Enums / Constants
Future features not in these PRs:
Original by SlashScreen
Resolves #356 ,
and expands terrain limit from 256 to 2048.(admin)