Crystal-Nest / harvest-with-ease

Don't break crops, right click them!
https://modrinth.com/mod/harvest-with-ease
GNU General Public License v3.0
5 stars 1 forks source link

Item entities spawn too high sometimes #32

Closed BioTechproject27 closed 7 months ago

BioTechproject27 commented 7 months ago

Describe the bug The item entities when harvested seem to spawn too high, e.g. when someone is standing ontop of a field, which causes them to spread a bit too far. They generally seem to spawn at head level.

Harvested standing ontop of the farmland: Left: Harvested by hand (breaking). Right: With a hoe (right-click). 2023-11-29_02 40 02

Harvested levitating on farmland level: 2023-11-29_02 42 19

System information:

Crystal-Spider commented 7 months ago

From which side did you right click the crop when right-click harvesting?

BioTechproject27 commented 7 months ago

From which side did you right click the crop when right-click harvesting?

When standing ontop of the farmland, mostly the top, except the back which was harvested by clicking on the side of the crop block. When hovering to the side, the side of the crop block.

I tested this after what you said, and yeah, it looks more like when right-clicking the top, the items spawn ontop of the crop-block (and thus way too high), while when clicking the side, they spawn inside the crop-block.

Crystal-Spider commented 7 months ago

I'm afraid this is not a bug, it is intentional that crops pop out of the right-clicked face.
This happens to prevent them from getting stuck in certain situations.

BioTechproject27 commented 7 months ago

I'm afraid this is not a bug, it is intentional that crops pop out of the right-clicked face. This happens to prevent them from getting stuck in certain situations.

Then they should also pop out the side, which they don't. They spawn inside the block that was harvested.

In which cases would they even get stuck?

BioTechproject27 commented 7 months ago

And even so, the spread is way too far. Annoyingly far. Surely there is a better way to solve such "stuck in block" problems? (And the current behavior wouldn't fix such behavior anyways, since when you click from the side the items will still spawn inside the crop block. I mean, surely one could run e.g. a collision check before deciding where to spawn items? But without knowing the problem nobody can effectively provide possible solutions.) Could you describe the situations?

Crystal-Spider commented 7 months ago

From the side:

https://github.com/Nyphet/harvest-with-ease/assets/65339023/1f0dc575-444b-425c-aa6e-e575bff4c8c9

From the top:

https://github.com/Nyphet/harvest-with-ease/assets/65339023/704c09e8-9c22-41cf-abe1-ae7d5bda17c4

As you can see, when I harvest from the side they do all drop out from the clicked side.
When harvested from the top, they all drop out from the top side.
So this does effectively solve the stuck-in-block problem.
I found out that problem when handling harvestable crops in the leaves of trees. Since the crop in that case is a block surrounded by other blocks, the drops would just collide like crazy and sometimes get stuck. If instead I make them pop out of the right-clicked face, I can be sure that such face is not in contact with any other block (because otherwise the player couldn't have right-clicked from there).
A collision check is much more inefficient than just saying from which face to pop out the drops, especially when multi-harvesting a large amount of crops.

The spread is just a block distance in the right-clicked direction (e.g. when clicking the side all the drops move into the block space to the side of the crop, when clicking the top they all move into the block space above the crop).

BioTechproject27 commented 7 months ago

As you can side, when I harvest from the side they do all drop out from the clicked side.

Yea after I tested again, they all spawned outside a possible hitbox (when I tested before it looked like they spawned inside the block as I didn't quite isolate them and lost track. With less crops it was much more visible (and thx to f3+b & carpet /tick freeze)) I'm sorry, I retract previously related statements.

But I also found that the items that spawn on the top face have double the motion upwards (0.2d, which is also what vanilla crops have in upwards motion, but the spawn in the middle of the block instead) compared to the ones spawning on the sides (0.1d), which perhaps increases the spread when the items land for "low" crops like wheat n such, but makes the item spread on full blocks (like mentioned leaves) much less. Kind of sucks for both ig but I see how you arrived at your comprimise.

I can be sure that such face is not in contact with any other block (because otherwise the player couldn't have right-clicked from there).

Although this is not quite true. Many blocks exist that have/can have a non-full, edge-touching face. Like walls/fences (unlikely to connect in the case of leaves) or things like stairs, slabs, cauldrons, snowlayers, gates and whatevernot (although yes, these would be rarer cases.)

Anyway, as to solutions: A collision check is performed for every late-stage mob spawn attempt, so is it really that bad?? Maybe one can spread that resource load over many ticks as to avoid lag spikes (e.g. make it a propagating 1-tick delayed wave, could also have a bonus satisfying effect? Although that may also cause rather funky bugs, like perhaps dupes if the crop is destroyed first, but these would be super rare cases I guess and not even OP in most cases. And to track all that to avoid crop dupes sounds like painful coding to me.) Or perhaps is there a way to check if the block has collision at all (e.g. blocks like grass/carrots/torches/open gates vs e.g. leaves, walls, closed gates) and then decide on that outcome where to spawn the item?

Man this crop yeeting just really bothers me

Crystal-Spider commented 7 months ago

I will try to find something that lessens the distance crops travel when popping out, but this is low priority for me, so I don't know when I'll actually do that. But eventually I will.

Crystal-Spider commented 7 months ago

It should be fixed in v8.0.0.1