Skip to content

Conversation

@OmeWillem
Copy link

Title says it all.

@duranaaron
Copy link

+1, I would really like this to be implemented as well

@OmeWillem
Copy link
Author

Any news on this being merged maybe?

@onebeastchris
Copy link
Member

I'm unsure whether this is, right now, a good API addition. Few issues:

  • This PR is currently wholly undocumented, which for API is generally terrible

  • This PR makes use of code - mainly the NonVanilaCustomBlockData class - that has an initially different purpose, hence there's some behaviour that wouldn't be triggered for it (which would be for CustomBlockData itself).
    That's not necessarily a blocker, but this PR lacks any description of e.g. an example mapping/usage that's possible with this change that isn't possible without it.

  • As mentioned above - it would be ideal if there would be a provided mapping example, with a clear description of what this PR enables (and what isn't possible without it). That way, we can properly test and review it, and account for this behaviour when we eventually refactor code. As an example, see Fix: Allow adding custom items to the creative item list to allow custom recipes to show up in the recipe book #4484 :)

In short - we're open to add features and are thankful for PRs. However, we'll need more details on what this enables doing that's not possible without it, so we can properly account for it when reviewing / when refactoring code in the future / when considering API design in a potential V2 :)

(final note - what's that collision check for? Does that issue still occur with the latest version of Geyser that registers collisions for custom blocks?)

@OmeWillem
Copy link
Author

Hi, I'll respond to your points:

  • Why would this need documentation? It simply allows for geyser_custom to be registered alongside the already accepted minecraft namespace.
  • The use of NonVanillaCustomBlockData allows for items to be registered alongside it. I have tried doing this with Minecraft blocks themselves, but that simply doesn't work. This means that you can add a new item using something like CustomModelData but make it behave like a block.
  • The only change needed is from the minecraft namespace to the geyser_custom namespace. Nothing else is changed from the regular. Everything else is still supported.
  • The collision check was added due to a NullPointerException.

@onebeastchris
Copy link
Member

onebeastchris commented Feb 16, 2025

This doesn't actually answer why you'd want to register a block with the same exact namespace as the item.

We document all mapping features on our wiki (not asking for a wiki PR, just for a PR description here that would give us more info so we can document it). This documentation is done so everyone using the mappings can know about all known functionality that can be used. Further, that ensures that users don't need to be able to read code to see how to utilise something properly.

As for the NPE, please re-test whether this occurs on the latest version of Geyser.

@onebeastchris onebeastchris added the Waiting On Response When an issue or PR is waiting on a response from a [specific] person. label Mar 3, 2025
@OmeWillem
Copy link
Author

Someone added documentation and the NPE did still occur without that null check. Is this to your liking or do we need to make a few changes?

@onebeastchris
Copy link
Member

onebeastchris commented May 20, 2025

After internal discussion, we've decided to not merge this PR in it's current state, and close it.

Currently, the Geyser custom block API isn't designed to register blocks "just" for the item, and not actual blocks that can be placed. If we add that feature, it'll be documented properly instead of the workaround proposed here. We're aware that it's already possible to this something similar to this via Extensions - but similarly as to non-vanilla items/blocks, some functionality won't be added to json mappings. Either due to complexity, undocumented behaviour, or to avoid user confusion.

Further:

Further, the PR description doesn't state what it's actually proposing to implement. This makes it impossible to review (without further context), as we aren't able to tell easily what the goal/expected behaviour is. As with every larger codebase, it's vital to be able to track who added which changes for which reason.

Thank you anyways!

@onebeastchris onebeastchris removed the Waiting On Response When an issue or PR is waiting on a response from a [specific] person. label May 20, 2025
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