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

Changed world_end to world_the_end #3531 #3538

Merged
merged 6 commits into from Feb 24, 2017
Merged

Changed world_end to world_the_end #3531 #3538

merged 6 commits into from Feb 24, 2017

Conversation

Bond-009
Copy link
Contributor

@Bond-009 Bond-009 commented Jan 15, 2017

@NiLSPACE
Copy link
Member

You should also change the linked worlds over.

@SafwatHalaby SafwatHalaby self-requested a review January 15, 2017 13:15
Copy link
Member

@SafwatHalaby SafwatHalaby left a comment

Choose a reason for hiding this comment

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

I think both _end and _the_end should be supported to prevent confusion for existing installations, with _the_end being the default. Doing this requires trivial changes.

@@ -376,11 +376,11 @@ void cRoot::LoadWorlds(cSettingsRepositoryInterface & a_Settings, bool a_IsNewIn
{
a_Settings.AddValue("Worlds", "DefaultWorld", "world");
a_Settings.AddValue("Worlds", "World", "world_nether");
a_Settings.AddValue("Worlds", "World", "world_end");
a_Settings.AddValue("Worlds", "World", "world_the_end");
Copy link
Member

Choose a reason for hiding this comment

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

This code block is fine and it does not affect existing installations.

@@ -451,10 +451,10 @@ void cRoot::LoadWorlds(cSettingsRepositoryInterface & a_Settings, bool a_IsNewIn
}
NewWorld = new cWorld(WorldName.c_str(), dimNether, LinkTo);
}
// if the world is called x_end
// if the world is called x_the_end
else if ((LowercaseName.size() > EndAppend.size()) && (LowercaseName.substr(LowercaseName.size() - EndAppend.size()) == EndAppend))
Copy link
Member

Choose a reason for hiding this comment

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

The problem here is that if a player removes their existing world_end, it would be recreated as an overworld called world_end. I think this code block should catch both _end and _the_end.

Copy link
Member

Choose a reason for hiding this comment

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

EndAppend1.= "_the_end"
EndAppend2 = "_end"
...
else if
(
	((LowercaseName.size() > EndAppend1.size()) && 
	(LowercaseName.substr(LowercaseName.size() - EndAppend1.size()) == EndAppend1))
	||
	((LowercaseName.size() > EndAppend2.size()) &&
	(LowercaseName.substr(LowercaseName.size() - EndAppend2.size()) == EndAppend2))
)

@@ -451,10 +451,10 @@ void cRoot::LoadWorlds(cSettingsRepositoryInterface & a_Settings, bool a_IsNewIn
}
NewWorld = new cWorld(WorldName.c_str(), dimNether, LinkTo);
}
// if the world is called x_end
// if the world is called x_the_end
Copy link
Member

Choose a reason for hiding this comment

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

"if the world is called x_the_end or x_end" (see below)

- Create a world.ini with a dimension type of "overworld".
- If a world called x_nether exists, set it as x's nether world.
- Otherwise set x's nether world to blank.h
- If a world called x_end exists, set it as x's end world.
- If a world called x_the_end exists, set it as x's end world.
Copy link
Member

Choose a reason for hiding this comment

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

"if the world is called x_the_end or x_end" (see below)


- If the world name is x (and doesn't end with _end or _nether)
- If the world name is x (and doesn't end with _the_end or _nether)
Copy link
Member

Choose a reason for hiding this comment

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

and doesn't end with _the_end or _end or _nether (see below)

- If the world name is x_end, create a world.ini with the dimension type "end".
- If a world called x exists, set it as x_end's overworld.
- Otherwise set the default world as x_end's overworld.
- If the world name is x_the_end, create a world.ini with the dimension type "end".
Copy link
Member

Choose a reason for hiding this comment

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

"if the world name is x_the_end or x_end" (see below)

@SafwatHalaby
Copy link
Member

This looks fine now. CircleCi is complaining though, and someone should perform ingame testing before merge.

if (cRoot::Get()->GetWorld(MyEndName) == nullptr)
MyEndName = "";
}

Copy link
Member

Choose a reason for hiding this comment

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

There's indentation by spaces (yuck) here, and missing braces and indent around the if-controlled block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


- If the world name is x (and doesn't end with _end or _nether)
- If the world name is x (and doesn't end with _the_end, _end or _nether)
- Create a world.ini with a dimension type of "overworld".
- If a world called x_nether exists, set it as x's nether world.
- Otherwise set x's nether world to blank.h
Copy link
Member

Choose a reason for hiding this comment

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

There's a stray "h" here.

{
MyEndName = GetName() + "_end";
if (cRoot::Get()->GetWorld(MyEndName) == nullptr)
MyEndName = "";
Copy link
Member

Choose a reason for hiding this comment

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

Indent missing.

@sphinxc0re
Copy link
Contributor

@Bond-009 Are you still working on this? Could you give us an update?

@Bond-009
Copy link
Contributor Author

I'm done with this, but it still needs testing.

@sphinxc0re
Copy link
Contributor

@Bond-009 There are still change requests to be fulfilled. Also please rebase your branch onto the current master branch of the canon repo

@Bond-009
Copy link
Contributor Author

The only change request that is not done is the one with the stray "h" and I don't about what he is talking.

@madmaxoft
Copy link
Member

There's an extra h on the line above that comment, not too relevant to this PR. Merging.

@madmaxoft madmaxoft merged commit ca3aa4c into cuberite:master Feb 24, 2017
@Bond-009 Bond-009 deleted the endsuffix branch February 24, 2017 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants