reworking stand animation stuff#29
Conversation
PlanetTeamSpeakk
left a comment
PlanetTeamSpeakk
left a comment
There was a problem hiding this comment.
Not a huge fan of the approach taken here to achieve different states having different sources for their animations. Imo, the source for an animation should be part of the animation state itself and should default to the stand's own animations rather than using a different source based on the state in the model.
| /** | ||
| * Determines if the current entity state should use base model animations | ||
| */ | ||
| private boolean shouldUseBaseModelAnimations(AbstractStarPlatinumEntity<?, ?> entity) { |
There was a problem hiding this comment.
Not a huge fan of this way of doing the separate animation sources. I think it'd be better to modify the interface state enums implement so that each state includes the source of its animation. More versatile and less prone to breaking.
| return createAnimationResource("base_model"); | ||
| } else { | ||
| return createAnimationResource("star_platinum"); | ||
| } |
There was a problem hiding this comment.
This creates a new instance of ResourceLocation every time its called (which probably happens a lot), consider caching it in a field instead.
However, if you go with my suggestion of moving this to the animation state interface, this won't be necessary regardless.
| * @return the animation resource for that stand | ||
| */ | ||
| protected ResourceLocation createAnimationResource(final String standName) { | ||
| return new ResourceLocation(type.getId().getNamespace(), "animations/" + standName + ".animation.json"); |
There was a problem hiding this comment.
Consider using type.getId().withPath
| List<ResourceLocation> allAnimations = getAllAnimationResources(entity); | ||
|
|
||
| // Return the primary animation resource (first in the list) | ||
| return allAnimations.isEmpty() ? animation : allAnimations.get(0); |
There was a problem hiding this comment.
Not much use to creating a list with all the additional animation sources if this method always simply uses the first one.
Classes that choose to override just this method can override it to implement all logic needed and wouldn't need to rely on getAllAnimationResources.
|
|
||
| public StarPlatinumModel(final boolean theWorld) { | ||
| super(theWorld ? JStandTypeRegistry.STAR_PLATINUM_THE_WORLD.get() : JStandTypeRegistry.STAR_PLATINUM.get()); | ||
| public StarPlatinumModel(boolean someCondition) { |
There was a problem hiding this comment.
If this parameter is no longer needed, please delete it.
|
most stands in the future are going to be using the bade model's animation shtuff for standby and other things maybe. this is the simplest approach i could think of, and it's also still in the state idk what youre talking about. i just made it so a stand can access multiple animation jsons at a time. |
|
information from: dumrole please: record |
2 participants
enabled standby on SP, made it so stands can use multiple animation files, reference StarPlatinumModel and StandEntity model as well as StarPlatinumEntity to see how i did it