BentoBoxWorld / Greenhouses

BentoBox Add-on to enable personal biomes in a glass greenhouse
Eclipse Public License 2.0
13 stars 9 forks source link

Allow omitting LocalMaterial in biomes.yml if you don't want a local blo… #79

Closed Rossterd closed 3 years ago

Rossterd commented 3 years ago

…ck to be required.

Same idea as Lekro, almost the same code

tastybento commented 3 years ago

Can you fix the test failure?

Error:    BiomeRecipeTest.testConvertBlockNotInGreenhouse:286 
block.setType(<any>);
Never wanted here:
-> at world.bentobox.greenhouses.greenhouse.BiomeRecipeTest.testConvertBlockNotInGreenhouse(BiomeRecipeTest.java:286)
But invoked here:
-> at world.bentobox.greenhouses.greenhouse.BiomeRecipe.convertBlock(BiomeRecipe.java:253)

Ideally, there will be a test with no LocalMaterial and one with it. If you don't know about tests then I can do it, but it'd be great if you can.

tastybento commented 3 years ago

Also, how does this solve the issue that Lektro had? Does it?

Rossterd commented 3 years ago

To be honest, I am new to almost everything github related.. I have some experience bodging code here and there but have only recently taken to programming in a more structured way. I saw the test fail on my other merge aswell but I wasn't quite sure what to do with it. It calls ConvertBlock for a block that isn't inside of a greenhouse expecting to get a rejection while the current Convertblocks logic makes it so that ConvertBlock will never be called on a block outside of a greenhouse. Adding a check would be unnecessary I think. But I wasn't certain enough to outright remove the test. (And removing tests just because they fail feels a bit weird ;) )

I'd happily take a look at making some tests for this, but I don't really know how it all works. Any chance you could point me in a direction?

Rossterd commented 3 years ago

38

Sorry about the long delay. I tested it, but apparently not thoroughly enough! It seems like leaving localMaterial = null implicitly requires air. Will try to investigate this soon

Originally posted by @lekro in https://github.com/BentoBoxWorld/Greenhouses/issues/38#issuecomment-570979270

It seems that the original block conversion logic took localBlock = null as air, it no longer does this so now this works.

tastybento commented 3 years ago

Thanks for the changes. I can try this later in the week.

Rossterd commented 3 years ago

Is there a problem?

tastybento commented 3 years ago

Sorry, I was very busy at real life work. I can merge this.