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

Optimizations to vanilla block models #2508

Closed
wants to merge 11 commits into from
Closed

Conversation

muzikbike
Copy link
Contributor

Fixes #2460.

Models optimized so far:

  • Stairs (straight and inner)
  • Hoppers (down and horizontal)
  • Composter
  • Chorus flower
  • Turtle eggs of stack size 4
  • Candle cakes
  • Anvil
  • Beacon
  • Item frame, with or without map
  • Hanging lantern
  • Hanging mangrove propagule

Models I'm still to do:

  • Dragon egg
  • Flower pots and potted objects
  • Cauldron

See #2460 for explanations of these optimizations as well as performance comparisons.

@mrjasonn
Copy link

mrjasonn commented May 19, 2024

Wouldn't doing ones that fix custom texture mapping break some custom resource packs which are adapted to the fact that they have broken texture mapping? If that is the case, shouldn't we just optimise it and remember to keep them looking the same as vanilla? (the looking same as vanilla should also be for everything in this PR)

@mrjasonn
Copy link

And if we are using the attached resource packs in the Mojiras to optimise the models, shouldn't we have a credits thanking them for their work?

@mrjasonn
Copy link

mrjasonn commented May 19, 2024

...and for the rest (item frames, propagules) nobody will notice the difference and if they do they won't care....

Please just make them look vanilla (I don't mind them looking different if clip in in spectator mode), as people might want them to actually look vanilla and not just ignore the fact it doesn't look vanilla. You have said that fixes for #1316 and #1902 may make this not visible, so please test on top of those commits. If it is harder to see, still best to keep vanilla in my opinion.

@jellysquid3
Copy link
Member

And if we are using the attached resource packs in the Mojiras to optimise the models...

As far as I know, this is Muzikbike's original work. If it's not, then we need to know, since we require that all contributions be the submitter's original work...

@jellysquid3
Copy link
Member

The changes seem fine to me. The minor details you mentioned are very unlikely to cause issues. But, for us to import these into the repository, each model file should probably have documentation (in the form of a __comment line at least) explaining what changes were made from Vanilla.

@jellysquid3
Copy link
Member

@muzikbike Do you have any update on this? I'd like to merge it if you think it's ready. The only remaining blocks that don't appear to be addressed by this pull request are not so common in the world anyways (Dragon Eggs, Flower Pots, Cauldrons...), so the performance impact of improving them is very, very small.

@djmrFunnyMan
Copy link

Apparently dragon eggs got optimized (MC-262652 was fixed) in the latest snapshot

@muzikbike
Copy link
Contributor Author

I'd prefer to finish optimizing the remaining blocks. Cauldrons are a major offender and show up in a good handful of farms, so those are a priority to fix.

@mrjasonn
Copy link

@jellysquid3, if this gets done in time, would this be something that potentially gets added in Sodium 0.6?

@jellysquid3
Copy link
Member

jellysquid3 commented Oct 19, 2024

Uhh... If the pull request was ready in the day or so, then yes, we could package it for Sodium 0.6.0. But the timeline on that is likely to be too close.

It's also not necessary to have all block models addressed in the initial pull request, if those need more time. We can always follow up with another pull request later down the road.

@muzikbike
Copy link
Contributor Author

I should be able to do cauldrons in a later PR if people want this in 0.6. What else needs done before I can consider this ready?

@jellysquid3
Copy link
Member

There is nothing blocking this pull request on our side. But while it is marked as a draft we cannot do anything with it.

@muzikbike muzikbike marked this pull request as ready for review October 22, 2024 22:58
Copy link
Member

@jellysquid3 jellysquid3 left a comment

Choose a reason for hiding this comment

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

There are many issues with inconsistent formatting and indentation in this pull request. Can you please run the files through a JSON formatter to normalize everything? We use 4-wide spaces in the project, so that should be used here too.

@muzikbike
Copy link
Contributor Author

Just threw everything into Blockbench and exported it. Does this suffice?

jellysquid3 added a commit that referenced this pull request Oct 26, 2024
@jellysquid3
Copy link
Member

I've merged the changes manually with d3aed21 to accommodate code style requirements, and because your branch was not tracking the multiloader changes.

Thank you for your efforts.

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.

Optimize some of vanilla's block models
4 participants