Sky Compass

[shartte]

This gets the sky compass into a working state. There is still area for improvement w.r.t. to how it is held in hand. Currently working:

• Pointer points in the right direction when held in hand
• Pointer points in the right direction when placed on ground

In all other cases, the pointer spins.

It uses two JSON models as the base (compass_base and compass_pointer) and combines them dynamically in a built-in IBakedModel subclass. The quads of the pointer are rotated using a custom QuadGatheringTransformer that applies a Matrix4f to the vertices. This is similar to what the CachingRotatingBakedModel does, but not equivalent. To get it to animate in the inventory and when held in hand, the aforementioned IBakedModel overrides IBakedModel.getOverrides and returns an ItemOverrideList that manipulates the IBakedModel's state based on who's holding it in handleItemState.

[Elix-x]

Well, i need to finish with renaming and then with cables or otherwise we will result with duplicated code xD.

To be rewritten post-cables (including TESR -> FMAS).

@dpeter99 all good for me.

[shartte]

To be rewritten post-cables (including TESR -> FMAS).

You can't set the time-value of the animation for held items as far as I know. But that's required to make it point in a specific direction and not just spin randomly. Same reason the FMAS is a bad fit for the crank. It just points in a specific direction sent by the server.

What I can do though is reuse the transformer stuff from this PR to make the crank use a Fast TESR.

[Elix-x]

Nah, no, it's ok. And it will definetly be rewritten by cables to use pipeline instead _{^{(something i'm working on with cables for a month already)}}.

[Elix-x]

9 add it to current one.

[shartte]

@elix-x Is your cable stuff shared somewhere so we can speed that up together?

edit: I think I'll otherwise run out of low hanging fruits soon. I'd go for facades, but that'll have to rely on your cable pipeline. I could do item models for cables though since those will most likely not depend on your pipeline, no?

[Elix-x]

Sadly, no. I don't even have it on me right now (it's /git stashed due to renaming). I'll redeploy after i run script today, fix conflicts and finish (i'm seriously 99% done, renaming made me pause it).

[shartte]

That sounds awesome! Looking forward to it

[shartte]

Working on the two rotation levels now. @yueh I was about to introduce a CompassRotationHelper and now I know I am not allowed to :-P

[shartte]

Allrighty, the "i am spinning because I've found something" animation is now one rotation per 0.5 seconds (looks a lot smoother ingame):

[yueh]

Is a helper for a single class not a bit overkill?

Also it is mostly about global helpers. If you need to encapsulate a helper into a local class, it is probably fine to add them as inner class. Just having them globally accessible is a temptation to abuse them. Like somebody has the great idea to reuse the class for the crank rotation because it works. Until someone refactors the helper away.

Or just look at forge. It ships like dozens of static helper classes, some starting with FML, some with Forge and some with random names. Of course with no documentation about what is actually available and does not need to be reinvented. So with having to reading every class in existing to look for a potentially already existing piece of code is simply not feasible.

[shartte]

@yueh In this particular case, I would've called it "CompassRotationHelper" and encapsulated the functionality of getting the effective rotation (including animation logic) for a given block pos in it. I've moved it into CompassBakedModel as a public static method for now, and reuse it from both the TESR and the CompassBakedModel since I need it in both.

If the mod used a different package structure (functional instead of technical), I would have made that helper class have package-only visibility as well.

Regardless, I've pushed the change and amended the PR.

ps: I also agree with you about global helper classes. Usually when adding a function that could use an existing global helper, one has already forgotten that it even exists.

[shartte]

This was also a good opportunity to verify that meteorites generate correctly:

[Elix-x]

This was also a good opportunity to verify that meteorites generate correctly:

I checked them already when i ported... It took me hours to approve :laughing:...

[shartte]

Can this be merged or does it need more work? I depend on the MatrixVertexTransformer in my drive implementation and would like to submit once this is merged.

[shartte]

Will rework this once the restructuring is done.