Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON Entity Animations #1476

Merged
merged 27 commits into from
Sep 5, 2024
Merged

Conversation

Gaming32
Copy link
Contributor

This adds support for entity animations written in JSON. There is an existing Blockbench plugin to export in this format.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! I believe this needs some work to make it more supportive of additional features that modders could use.

@ChampionAsh5357 ChampionAsh5357 added enhancement New (or improvement to existing) feature or request rendering Related to rendering data driven This request involves a data driven system 1.21.1 Targeted at Minecraft 1.21.1 labels Aug 20, 2024
Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

Very nice! Mainly just documentation comments.

@Gaming32
Copy link
Contributor Author

Maybe you could also add support for json entity models?

That is completely out of scope for this PR.

@neoforged-automation
Copy link

@Gaming32, this pull request has conflicts, please resolve them for this PR to move forward.

@XFactHD XFactHD merged commit 0f411dd into neoforged:1.21.x Sep 5, 2024
6 checks passed
@neoforged-releases
Copy link

🚀 This PR has been released as NeoForge version 21.1.42.

@Tslat
Copy link

Tslat commented Sep 8, 2024

uhhhh

You used the same asset folder as GeckoLib, and this is now causing the game to absolutely spew errors when GeckoLib animations are present

com.google.gson.JsonParseException: Not a json array: {"misc.idle":{"loop":true,"animation_length":3.52,"anim_time_update":"0","blend_weight":"1","bone......... etc.

@lukebemish
Copy link
Contributor

lukebemish commented Sep 8, 2024

I feel like both this and geckolib should be using appropriately namespaced asset folders -- neoforge/animations/entity and geckolib/animations/entity. Yes, changing that on the neo end is technically a breaking change but if neo broke geckolib then the original change was also breaking, so it's fine to break to fix the original break.

@dhyces
Copy link
Contributor

dhyces commented Sep 8, 2024

I feel like both this and geckolib should be using appropriately namespaced asset folders -- neoforge/animations/entity and geckolib/animations/entity. Yes, changing that on the neo end is technically a breaking change but if neo broke geckolib then the original change was also breaking, so it's fine to break to fix the original break.

Geckolib should have been using a namespaced directory, yes. Neoforge apparently does not care as evidenced by datamaps using a non-namespaced folder.

@lukebemish
Copy link
Contributor

lukebemish commented Sep 8, 2024

Datamaps are namespaced, however -- they're namespaced by the thing actually registering the datamap, just like registries Apparently i misspoke. That's... Really terrible, wow. Those should also be namespaced I feel like...

@lukebemish
Copy link
Contributor

lukebemish commented Sep 8, 2024

Just following up on that -- datamaps are namespaced and that namespace is obeyed, neo just sticks its own datamaps in the default namespace for some reason. I misunderstood what was going on it seems -- data maps are, indeed, namespaced.

@Gaming32
Copy link
Contributor Author

Gaming32 commented Sep 8, 2024

I feel like both this and geckolib should be using appropriately namespaced asset folders -- neoforge/animations/entity and geckolib/animations/entity. Yes, changing that on the neo end is technically a breaking change but if neo broke geckolib then the original change was also breaking, so it's fine to break to fix the original break.

As I would like to PR this same feature to Fabric, it would be ideal if the directory was not put under "neoforge". If it was, it would be extremely inconvenient to both resourcepack developers and multiloader mod developers.

@lukebemish
Copy link
Contributor

Unless we can guarantee that fabric and neo would keep the same format and features for the feature, like they're in theory supposed to for convention tags, that's really irrelevant -- I do not believe that conversation has happened yet.

@TelepathicGrunt
Copy link
Contributor

Linking relevant Blockbench plugin PR that modders will be able to use to get the json for NeoForge to load:
JannisX11/blockbench-plugins#602

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.1 Targeted at Minecraft 1.21.1 data driven This request involves a data driven system enhancement New (or improvement to existing) feature or request rendering Related to rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants