ldtteam / minecolonies

Minecolonies minecraft mod
http://minecolonies.com/
GNU General Public License v3.0
609 stars 322 forks source link

Rework the quarrier to support other builder modes in the future #9967

Closed MotionlessTrain closed 1 week ago

MotionlessTrain commented 1 month ago

Depends on https://github.com/ldtteam/Structurize/pull/670 Closes # Closes #

Changes proposed in this pull request:

An oddity I noticed was that the quarrier would request materials to place entities, but doesn't have a spawn entity phase. I don't think there is any quarry with entities in it, but I'm wondering whether that would not need to be addressed (either by adding a fourth stage, or by removing the entities from the requestMaterials step)

[x] Yes I tested this before submitting it. [ ] I also did a multiplayer test.

Review please

uecasm commented 1 month ago

(I haven't looked at the code, just the description) Does it slice the bounding box to give successive 1y-high boxes to the underlying iterator, or does it hand the full box to the underlying iterator and pause whenever it changes y-level? Because the latter wouldn't work for the inwardcircleheightN iterators.

MotionlessTrain commented 1 month ago

The latter. The size is apparently only set during construction of the iterator based on the blueprint data in the given IStructureHandler, and it looked undoable to override the size of the inner iterator to only have one Y-level

I did not think about how the inwardcircleheight iterators are supposed to work with this. That is an interesting case

uecasm commented 1 month ago

It's possible, but not easy. You'd have to slice the blueprint itself into smaller blueprints and pass that through, as well as offsetting the returned y-value back to the full blueprint for actual use.

Or change the API on the Structurize side first to make it easier.

MotionlessTrain commented 1 month ago

I shortly looked into it, and it didn't seem easy to change the size on structurize's side in several iterators without breaking things (e.g. Hilbert and Random may break in some cases, it looked like)

I guess I do indeed need to modify the blueprint to solve this. I tried to avoid that, as it looked like a very complicated solution

Raycoms commented 1 month ago

Yeah, I would disable entities from the resources step for now for the quarry, so that both sides fit.