Removed some not needed code. Moved the default mcr/mca worlds' spawn in the center of the r.0.0.mc* region file #27
Conversation
… in the center of the r.0.0.mc* region file $diffX,Y,Z variables were not needed since they were the same as $dx,y,z with an opposite sign and were used as a power of 2 ( $diffX,Y,Z ** 2 ) that's always positive. $diffY was also used in a condition, to check if the value was inside the interval ( -$yS < $diffY < $yS ) , so, the sign is not needed to check if it's inside this interval. The default mcr/mca spawn was not centered in the 0,0 region causing the generation of not needed regions walking just a bit
|
Please try to keep your changes to separate commits/PRs. |
|
@dktapps Ok, sorry. Should i make a new PR ? |
|
Leave it this time, the changes are small enough to be reviewed easily. But for future reference please keep changes separate. |
| @@ -85,9 +85,9 @@ public static function generate($path, $name, $seed, $generator, array $options | |||
| "initialized" => new ByteTag("initialized", 1), | |||
| "GameType" => new IntTag("GameType", 0), | |||
| "generatorVersion" => new IntTag("generatorVersion", 1), //2 in MCPE | |||
| "SpawnX" => new IntTag("SpawnX", 128), | |||
| "SpawnX" => new IntTag("SpawnX", 255), | |||
There was a problem hiding this comment.
Shouldn't it be 256, if the old was 128 (not 127)?
There was a problem hiding this comment.
A region contains 32 * 32 chunks.
A chunk is 16 * 16 * 128.
16 * 32 / 2 - 1 = 255
There was a problem hiding this comment.
Then why was it 128 before?
There are 512 blocks horizontally, so you want the coordinate between the 256th and 257th, i.e. index 255 and 256. The middle of the index-255 block is 255.5, so this should be 256.
| @@ -1356,16 +1356,12 @@ protected function processMovement($tickDiff){ | |||
|
|
|||
| $this->move($dx, $dy, $dz); | |||
|
|
|||
| $diffX = $this->x - $newPos->x; | |||
| $diffY = $this->y - $newPos->y; | |||
There was a problem hiding this comment.
$diffY is equivalent to -$dy not $dy
There was a problem hiding this comment.
that's what i said 😕
| $yS = 0.5 + $this->ySize; | ||
| if($diffY >= -$yS or $diffY <= $yS){ |
There was a problem hiding this comment.
The original code seems to have logic problems. I think it should be an and operator such that it can be expressed as -$yS <= $diffY <= $yS mathematically
| $yS = 0.5 + $this->ySize; | ||
| if($diffY >= -$yS or $diffY <= $yS){ | ||
| $diffY = 0; | ||
| if(-$yS <= $dy or $dy <= $yS){ |
There was a problem hiding this comment.
So -$yS <= $dy and $dy <= $yS can be expressed as -yS <= dy <= yS mathematically, which is much more reasonable ($yS > 0 assumed). Otherwise, this statement is always true.
|
@SOF3 I can't add commits to this pull request, i deleted my fork . Is there another way? |
|
@svilex That's very bad practice. You should always retain it until the PR is merged. Deleting it simply makes it unchangeable. |
|
@svilex close this pull request. Your new fork still shares the same Git tree, so you can still create a new branch in your new fork and merge the new commit 38d328a into your branch: git clone https://github.com/svilex/PocketMine-MP.git --recursive && cd PocketMine-MP
git checkout 38d328a31053399f0968a2dec63dc943ba62167c
git checkout -b my-new-branch
git push -u origin my-new-branch |
|
Edit: Better not merge this commit directly. Make two new pull requests instead. |
ghost
mentioned this pull request
$diffX,Y,Z variables were not needed since they were the same as $dx,y,z with an opposite sign and were used as a power of 2 ( $diffX,Y,Z ** 2 ) that's always positive. $diffY was also used in a condition, to check if the value was inside the interval ( -$yS < $diffY < $yS ) , so, the sign is not needed to check if it's inside this interval.
The default mcr/mca spawn was not centered in the 0,0 region causing the generation of not needed regions walking just a bit