Skip to content

tester#40

Open
z2six wants to merge 1 commit into
ldtteam:version/mainfrom
z2six:version/main
Open

tester#40
z2six wants to merge 1 commit into
ldtteam:version/mainfrom
z2six:version/main

Conversation

@z2six

@z2six z2six commented Jan 31, 2026

Copy link
Copy Markdown

Hi there,

Changes are:

  • Hot-loaded server config for adding block ID's to override whether they can be pushed by Multipiston
  • Commands that do the same
  • Works for me on odd blocks like the Iron Porticullis

@CLAassistant

CLAassistant commented Jan 31, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Raycoms Raycoms left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the problem with MOVEABLE_ENTITY_BLOCKS?

You can hot reload the tags as well by reloading datapacks?

@Raycoms

Raycoms commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

And thanks for the PR first and foremost =)

@z2six

z2six commented Jan 31, 2026

Copy link
Copy Markdown
Author

Hi Raycoms,

It's up to you really, but two reasons:

  • The hotloaded server config + commands add on top of the existing blocktags. I don't like Datapacks, this is more accessible to me and my server admins who don't have direct access to my server that's running. Maybe there are more people like me.
  • Blocktags will still be subject to piston push-reaction checks. The changes in my logic circumvent that.

@Raycoms

Raycoms commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

Hi Raycoms,

It's up to you really, but two reasons:

  • The hotloaded server config + commands add on top of the existing blocktags. I don't like Datapacks, this is more accessible to me and my server admins who don't have direct access to my server that's running. Maybe there are more people like me.
  • Blocktags will still be subject to piston push-reaction checks. The changes in my logic circumvent that.

What do you mean with the second?

@Thodor12

Copy link
Copy Markdown
Contributor

In all honesty, not liking datapacks is not a reason to add this much code to achieve something that is already achievable. On top of that you're running into other issues now.

  • What if a server owner does not want this command enabled at all?
  • What if a server owner wants to change the permission level of the command so i.e. only himself can run it (permission level 4)?
  • What if people have datapacks already installed, the list command is not going to list those items, so this is going to cause a weird disconnect between what's actually possible vs what is listed through the commands.
  • There is no kind of limit checking to the configuration, if people add a ton of different resourcelocations (not blocks), will it fail/become slow at some point?
  • The resource locations you type aren't verified if they are valid blocks, nor a cleaned up if said blocks no longer exist. They can just continue to pile up into infinity.

@z2six

z2six commented Jan 31, 2026

Copy link
Copy Markdown
Author

Quick responses, love it!

@Raycoms I'm referencing the blockToMove.getPistonPushReaction() == PushReaction.IGNORE/DESTROY/BLOCK gate inside the handleTick() method. It's great to have and to keep. But, if server admins would want to make e.g. Obsidian moveable (or in my case, many of the blocks in my kitchen sink modpack), a datapack would not have explicit override of said gate.

@Thodor12 Thanks for the pointers, good feedback. I could implement e.g.:

  • Config options for enabling commands, permission level
  • Block ID validation when using commands
  • Set a hard cap on the list size
  • Add a list-all that merges both the config list and datapack list

The config I've added is supposed to be additive, not a replacement. 😄 Also, the Neo branch does not currently have these tags as I mentioned before.

Again, it's up to you guys. It's a small change that was necessary for me. I'd prefer it to be pulled in the official repo, but it does not matter much to me in all honesty. Feel free to close it if you feel it doesn't fit in your architecture or conceptual choices. Either way, thanks again for the mod, definitely enjoying my portcullis 😉

@Raycoms

Raycoms commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

Oh ye, on 1.21 it's on the release branch, I derped that.

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.

4 participants