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

Create PR, fixing tons of bugs #18

Merged
merged 9 commits into from May 13, 2017
Merged

Create PR, fixing tons of bugs #18

merged 9 commits into from May 13, 2017

Conversation

inxomnyaa
Copy link
Collaborator

WIP, do not merge yet

@inxomnyaa
Copy link
Collaborator Author

Done, can be merged.

@@ -91,7 +92,7 @@ public function generateCave($x, $y, $z, Random $random) {
foreach($gen = $this->generateBranch($x, $y, $z, 5, 3, 5, $random ) as $v3 ) {
$generatedBranches --;
if ($generatedBranches <= 0) {
$gen->send(self::STOP);
$gen->send(self::STOP); // send not found.. @Ad5001 what is that

Choose a reason for hiding this comment

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

where is send() at?

Copy link
Owner

Choose a reason for hiding this comment

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

@thebigsmileXD @jasonwynn10 http://php.net/manual/en/generator.send.php
It was my first generator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the function is missing and leads into alot of issues

Copy link
Owner

Choose a reason for hiding this comment

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

It's not missing, the generateBranch function is a generator (usage of yield keyword). All functions related to Generators can be found on PHP.net

Copy link
Collaborator Author

@inxomnyaa inxomnyaa May 13, 2017

Choose a reason for hiding this comment

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

i looked into the yield. but stop/start are missing still.. could it be they are not supported by php7?

break;
}elseif($b !== Block::AIR){

Choose a reason for hiding this comment

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

what about transparent blocks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You don't want to replace transparent blocks ;) (glass, water, pressure plates, doors...)

$fallenTree = new FallenTree(new $tree());
$fallenTree = new \Ad5001\BetterGen\structure\FallenTree(
new $tree()
);

Choose a reason for hiding this comment

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

why change the formatting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1st: a class error. Second: because the whole formatting was messed up anyways. Here it actually was a "debug" to get the correct error line (Cannot use tree as tree because already...), but fixed though

@mkdir($this->getDataFolder());
if(! file_exists(LootTable::getPluginFolder(). "processingLoots.json"))
file_put_contents(LootTable::getPluginFolder(). "processingLoots.json", "{}");

Choose a reason for hiding this comment

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

the doc comment needs changed

Copy link
Owner

Choose a reason for hiding this comment

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

+1

@Ad5001
Copy link
Owner

Ad5001 commented May 13, 2017

To explain this formating, I like 1 time opened the project with eclipse which decided to format it all !

@Ad5001 Ad5001 merged commit d954937 into Ad5001:master May 13, 2017


//TODO CHECK WHAT THIS IS SUPPOSED TO BE
$numForward = ($totalLength % 2 == 0) ? $totalLength - 1 : $totalLength;
Copy link

Choose a reason for hiding this comment

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

this is not merge ready unless this is known

Copy link
Owner

Choose a reason for hiding this comment

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

This is known. This is the length of the leaves falling forward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

idk why i left this in, i already fixed it

@inxomnyaa
Copy link
Collaborator Author

inxomnyaa commented May 13, 2017

To explain this formating, I like 1 time opened the project with eclipse which decided to format it all !

You don't format with eclipse. Rule no 1 of coding xd

@AvgZing
Copy link

AvgZing commented May 13, 2017

Never format with eclipse. Rookie mistake

@Ad5001
Copy link
Owner

Ad5001 commented May 13, 2017

I did not choose. It's been done automaticly. That's the reason I deleted it.

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

4 participants