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

Fix multi-part model inventory rendering #4629

Merged
merged 1 commit into from Jan 7, 2018

Conversation

boq
Copy link
Contributor

@boq boq commented Dec 27, 2017

After #4268 multi-part models (ones with submodels entry in blockstate) are rendered with transformation applied twice. This can be observed for example with test models from animation debug mod (spinning engine-thing) - in inventory it's smaller than expected and rotated at weird angle.

Variant state (defined in transform in JSON) currently is passed to model here (comment is invalid, state is applied to sub-models much later, during baking) and later used as IModel.getDefaultState return, but is also applied here - directly from variant.

Since other custom models don't know about variant transform, I'm removing that information from MultiModel and keep it applied during WeightedRandomModel construction - that way transform is applied to all models.

Other models (custom or vanilla) are unaffected, since they pass through ForgeVariant.process unaffected (see here).

private final Map<String, Pair<IModel, IModelState>> parts;

// for binary compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

If these aren't intended to be kept, except temporarily for compatibility, can you deprecate them and add a TODO: remove comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whole class is deprecated - for at least 3 major MC versions. But I can add TODO.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the whole thing is deprecated, then it's not such an issue.

@mezz mezz added 1.12 Bug This request reports or fixes a new or existing bug. labels Dec 27, 2017
@mezz mezz added the Assigned This request has been assigned to a core developer. Will not go stale. label Jan 5, 2018
@LexManos LexManos merged commit 10b8d47 into MinecraftForge:1.12.x Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.12 Assigned This request has been assigned to a core developer. Will not go stale. Bug This request reports or fixes a new or existing bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants