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

Made world data paths adjustable, and added API to temporarily disable saving chunks to disk. #3912

Merged
merged 18 commits into from Sep 7, 2017

Conversation

lkolbly
Copy link
Contributor

@lkolbly lkolbly commented Aug 8, 2017

This PR makes the data path that world data is saved in adjustable at world creation time (via constructor parameter and settings.ini parameter), which will be useful for restructuring the world files (as in #3426). The Anvil format, player stats, and maps are all modified to accommodate this change.

It adds the optional [WorldPaths] section to the settings.ini file, which specifies the directories that various worlds are stored in, for example:

[WorldPaths]
world=Worlds/overworld
world_nether=Worlds/nether
world_the_end=Worlds/end

This PR also adds the SetSavingEnabled method to cWorld and cRoot, which permits disabling saving chunk files to disk (note that it doesn't prevent writing the ini file to disk). This allows implementing the /save-off and /save-on commands. It will also allow easily resetting maps, I think will be useful for minigames (resetting the map then just becomes an issue of reloading the world).

m_SavingEnabled is atomic, so that cRoot can access it from its thread safely for console commands.

Tweaked the HOOK_CHUNK_UNLOADING documentation to say that the ability for the plugin to actually prevent the chunk from unloading is a feature not a bug.

@madmaxoft
Copy link
Member

I'm not too happy about having the world config split into two separate sections. But I can't think of a better way.

@lkolbly
Copy link
Contributor Author

lkolbly commented Aug 9, 2017

Ditto. On further thought, maybe another idea would be:

[Server]
...
DefaultWorld=world

[Worlds]
world=Worlds/world
world_nether=Worlds/world_nether

Where [Worlds] just specifies the paths, but then the [Server] section gets muddied with the DefaultWorld tag.

@lkolbly
Copy link
Contributor Author

lkolbly commented Aug 9, 2017

The [WorldPaths] is optional, so it should only appear in the rare instances where the paths are not the default. (the default for name would be Worlds/name for the architecture change)

@madmaxoft
Copy link
Member

madmaxoft commented Aug 9, 2017

Migration to that format would be non-trivial.

If only we already had Lua-based config files, we could simply do:

Worlds =
{
  world =
  {
    Path = "Worlds/world",
    SomeOtherParam = 3.1415,
  },
  -- ...
},
DefaultWorld = "world",

@lkolbly
Copy link
Contributor Author

lkolbly commented Aug 9, 2017

If I make the config files lua-based (in a future PR), can I make the crafting/brewing/etc files lua-based too?

@madmaxoft
Copy link
Member

Yes, but please discuss this first in the forum, we have had some discussions there already and there are some ideas that you might want to consider before diving in :)

@@ -8,8 +8,7 @@ return
Cuberite calls this function when a chunk is about to be unloaded from the memory. A plugin may
force Cuberite to keep the chunk in memory by returning true.</p>
<p>
FIXME: The return value should be used only for event propagation stopping, not for the actual
decision whether to unload.
CAUTION: Preventing the server from unloading chunks can cause the server to use too much RAM, which will adversly affect both performance and stability (i.e. your computer will get slow and crash). Return true sparingly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adversely

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today I learned.

src/Root.h Outdated
@@ -137,6 +137,9 @@ class cRoot
/** Saves all chunks in all worlds */
void SaveAllChunks(void); // tolua_export

/** Sets whether saving chunks is enabled */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for all worlds

src/World.h Outdated
@@ -111,6 +112,12 @@ class cWorld :

// tolua_begin

/** Get whether saving chunks is enabled */
bool GetSavingEnabled(void) const { return m_SavingEnabled; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to IsSavingEnabled and m_IsSavingEnabled

src/World.h Outdated
@@ -871,6 +881,9 @@ class cWorld :

AString m_WorldName;

/** The path to the root directory for the world files */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't contain the trailing path separator.

@lkolbly
Copy link
Contributor Author

lkolbly commented Aug 17, 2017

Alright, made some fixes and pulled the latest master.

I'll start working on Lua config files once all the file architecture dust settles. :-)

@lkolbly
Copy link
Contributor Author

lkolbly commented Aug 18, 2017

Alright, now it passes CI.

@bearbin
Copy link
Member

bearbin commented Sep 7, 2017

Resolved the conflict, just waiting for CI to pass then merge.

@bearbin bearbin merged commit b12f4ef into cuberite:master Sep 7, 2017
@lkolbly lkolbly deleted the worldDataPaths branch September 7, 2017 18:11
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

Successfully merging this pull request may close these issues.

None yet

3 participants