Skip to content
This repository was archived by the owner on Dec 13, 2023. It is now read-only.

Added compatibility with tall blocks#3

Merged
mymindstorm merged 5 commits into
mymindstorm:masterfrom
P0stf1x:master
Aug 21, 2020
Merged

Added compatibility with tall blocks#3
mymindstorm merged 5 commits into
mymindstorm:masterfrom
P0stf1x:master

Conversation

@P0stf1x

@P0stf1x P0stf1x commented Aug 1, 2020

Copy link
Copy Markdown
Contributor

Partial fix of #2
Don't work with doors and blocks that require silk touch

@mymindstorm

mymindstorm commented Aug 1, 2020

Copy link
Copy Markdown
Owner

Thanks for the PR!

Ideally, I'd rather not use a hardcoded method like this. What do you think about this:

  • Adding a custom field to the player that records the position of the last telekinesis block break
  • Getting all players within block break distance every time a block is broken with air (what sugar cane reports for stack when it is broken by the block below it, I still have to test other blocks)
  • Then, check whether any player's last telekinesis block break was within one block and one or two ticks from the last block break

This should solve the silk touch problem, eliminate the need to whitelist blocks, and solve cases like breaking the sand under a sugar cane or breaking the block behind a torch. I can try implementing this if you don't have the time.

@P0stf1x

P0stf1x commented Aug 2, 2020

Copy link
Copy Markdown
Contributor Author

About hard-coded blocks
I thought you might be already working on config file and it would be just waste of time I could bugfix the code.

I could try using NBT tags for players today or tomorrow, but I think it wouldn't work with choruses. But still better than that we have right now.

@mymindstorm

mymindstorm commented Aug 2, 2020

Copy link
Copy Markdown
Owner

I don't understand what you are referring to as a waste of time.

I don't think we need to persist any data as an NBT tag, the data only needs to be in memory (I.e. add a property and some methods to PlayerEntity via mixin). I am not working on a config file. We might have to just manually code in chorus plant logic.

@P0stf1x

P0stf1x commented Aug 3, 2020

Copy link
Copy Markdown
Contributor Author

Waste of time if we both separately added config support. If the mod will use different approach for tall blocks then config file isn't needed.

I've done most tall blocks support via checking if entity is null. Side effect of this is for some reason blocks still drop their item. I'll try to check once more, but maybe you could find how to fix this? Possibly delete item entity after it's spawned

@P0stf1x

P0stf1x commented Aug 3, 2020

Copy link
Copy Markdown
Contributor Author

I've checked most combinations, and the only thing isn't affected is torches placed on blocks. But is this an issue? For example, Hypixel Skyblock doesn't even add sugarcane to your inventory if you break block below it

@mymindstorm

Copy link
Copy Markdown
Owner

You need to return false to have it not drop a block. The torch thing isn't too much of an issue, I can take a look at it when I have the time.

@P0stf1x P0stf1x changed the title Added compatibility with tall flowers/crops Added compatibility with tall blocks Aug 4, 2020
@mymindstorm

mymindstorm commented Aug 10, 2020

Copy link
Copy Markdown
Owner

Off the top of my head there are two problems with the current implementation here:

  • What if two players are trying to break the same block? (E.g.: A breaks a block, but since B is closer to the block B gets the drop)
  • What about blocks that break without player interaction? E.g. leaves on a tree breaking

I've gone ahead and made some changes, what do you think of d187867?

@mymindstorm mymindstorm merged commit 1d4a4be into mymindstorm:master Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants