AEModernMCPort / Applied-Energistics-3-Fork

A Minecraft Mod about Matter, Energy and using them to conquer the world..
http://ae-mod.info/
Other
37 stars 12 forks source link

Implemented interface states and models #66

Closed shartte closed 8 years ago

shartte commented 8 years ago

Implemented interface model and directed interface arrows.

Introduced a new property type that can hold an AEPartDirection, because only that contains "INTERNAL", which I needed for the omnidirectional interface.

This relies on setting blockstates from tile entites and persists the block state in meta, since its big enough to hold the directional information.

Since the model already rotates correctly based on the point_at property, I disabled the FORWARD/UP properties for the blocks and made the caching rotating model skip such blocks.

This should fix #26

Elix-x commented 8 years ago

This relies on setting blockstates from tile entites and persists the block state in meta, since its big enough to hold the directional information.

Since the model already rotates correctly based on the point_at property, I disabled the FORWARD/UP properties for the blocks and made the caching rotating model skip such blocks.

As we decided with @yueh, ALL blocks should be rotatable on all axes. Including ones for which it makes no sense.

yueh commented 8 years ago

I am neither really sure about using AEPartLocation nor a Property for these data.

AEPartLocation has the semantic of where a part is, not where it is pointing at or even designed for being used with blocks. Also the interface is not pointing at the center or internal, but actually everywhere. It is more like directional && pointingAtFace. So it could also use a bool + EnumFacing. In general there are a few important differences between a undirectional and directional interface regardless of the direction it faces.

But the Property is the main issue. The interface already serialises a bunch of data, why introduce the facing as exception to it? Also the direction is very frequently used for the tile entity, easily in the range of several thousand calls per tick. While the property should only be used once on a chunk rerender. Going through the block, propertymap and what not each time is simply not ideal. Also it is needed a few other locations like for the IGridBlock. We could cache it. But then we have to deal with multiple sources of truth, correctly invalidating 3 or more caches and so on. As we already serialise all of the required data (short of being directional or not) through the tile entity, why not use an IUnlistedProperty and use the tile entity as SSOT?

Elix-x commented 8 years ago

@yueh little remark: IUnlistedProperty can't be passed to jsons, but a property from getExtendedState can (though they don't make much of difference).

yueh commented 8 years ago

Did forge not announce you can use them in their own json format or something similar?

But it is mostly about serialising the direction as redundant metadata.

Elix-x commented 8 years ago

Did forge not announce you can use them in their own json format or something similar?

You can use them in custom IBakedModels. Last time i tried with json, it didn't work, but i'll double check.

yueh commented 8 years ago

Also keep in mind that there is still the feature request for colourable machines. So something like a green north-facing interface will become a thing. (The feature literally needs the rendering part, functionality already exists)

Elix-x commented 8 years ago

Also keep in mind that there is still the feature request for colourable machines. So something like a green north-facing interface will become a thing. (The feature literally needs the rendering part, functionality already exists)

You said that just in time... So i need to apply the way i recolor cables to machines. Should be straight forward. @shartte no need to do that part yet.

yueh commented 8 years ago

Just to be careful, it should only recolour some faces. Like the cable itself, but not a part. Except on a part it should recolour the status light or things like the terminal colour.

But no idea for a "green interface". Recolour the whole thing? One or two of the dark rings? Just the center and maybe the arrows for directional ones? Same also applies to drives, assemblers, io ports and what not. Iirc chests and crafting monitors can already be recoloured, but that only applies to the colour of the monitor itself, not the grid colour.

Elix-x commented 8 years ago

That's completely up to texturer/modeler to decide. Hence, 100% RP compatible.

shartte commented 8 years ago

I will revise the PR to use the usual suspects (forward, up) and a boolean Property to decide omnidirectional=true|false for selecting the Model. I am sonewhat concerned about the useless duplicate Models we'll be auto-generating for the omnidirectional variant, but I assume we can optimize that later. I do hope we are not gonna make the user cycle through 36 interface states (6 * 6) of which 30 are superfluous however. I also only used the aepartdurection enum, because it was already abused in that way in TileInterface.

tl;dr will amend PR

On 18 Aug 2016 12:04 p.m., "Elix_x" notifications@github.com wrote:

That's completely up to texturer/modeler to decide. Hence, 100% RP compatible.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AEModernMCPort/Applied-Energistics-2/pull/66#issuecomment-240680102, or mute the thread https://github.com/notifications/unsubscribe-auth/ABM_V54nLU2z-56_65VIxIB-CSTLevCcks5qhC4agaJpZM4Jm-02 .

yueh commented 8 years ago

The rotations for interfaces always handled a bit different due to the directional/undirectional. It just cycles through 3 states, facing towards the side clicked at, the opposite and then back to undirectional.

I have not really looked at the rotation methods for blockstates. But it would probably a better idea to handle it there, even when internally saving all 24 permutations. Afaik most wrenches in 1.8+ already use them to rotate things. So it can provide a generic way instead of potentially having to support every wrench out there and let it behave correctly.

Elix-x commented 8 years ago

Afaik most wrenches in 1.8+ already use them to rotate things. So it can provide a generic way instead of potentially having to support every wrench out there and let it behave correctly.

No, from 1.9, they use rotation methods provided by vanilla, as vanilla never provided even 6 sides facing property (in 1.8, they had to guess property by name and valid values by blind trying).

yueh commented 8 years ago

I use 1.8+ more as the current minecraft version. This functionality is simply there, but it is not important, if it was added with 1.8.0 , 1.8.9 or 1.9.4. The basic technology has not really changed that much. Something like ^1.8.0 would probably be more fitting.

I am not even sure, if we should target 1.10 or 1.11. Minecon is just a month away and we might already see a pre release with it. 1.11 will also have a few breaking changes. For example it finally gets rid of all public fields. Finally everthing is access through setters and getters. Like ItemStack.increaseStackSizeBy(42) or so. Or the item field within ItemStack will finally be final.

Elix-x commented 8 years ago

Forge won't come until 1.1 is released. And mojang probably won't release 1.11 right away on minecon, so it will come soon, but not very soon.

Speaking of breaking changes, we already know that they enforced lower casing and changed a bunch of stuff in entities (at least subtypes). Luckily, second one does not affect us.

shartte commented 8 years ago

Okay I amended the PR and touched up TileInterface more than the initial PR was intending to do.

I've removed pointAt from TileInterface and replaced with a simple "omniDirectional" toggle. The interface will now use getUp() for the direction it's pointing and omniDirectional as a toggle between the two JSON models. Rotation is fully handled by the existing mechanism now.

I also added the update packets that were missing so that the omniDirectional state is correctly updated to clients.

Elix-x commented 8 years ago

getForward is the one you should use, not getUp :wink: .

Also, all we need is sync TE data, this is a work for another day as requires of touching far not only interface (so syncing can be left for now).

shartte commented 8 years ago

@elix-x Why delay the syncing? It works! And at least you can test this stuff now. Otherwise you never see the omnidirectional state again.

Regarding up/foward: It has to be up because in "resting" state that's how the oriented model is designed. And there's no functional difference with regards to rotation whether its up or forward.

Elix-x commented 8 years ago

Well, while we accept temporary solutions, they all have to be cleaned up after. It's ok, but it's you again (who will clean up).

because in "resting" state that's how the oriented model is designed

What? Please elaborate.

And there's no functional difference with regards to rotation whether its up or forward.

Yes, but, forward is called forward for a reason after all.

shartte commented 8 years ago

Well by default the oriented model points "up", but I'll change it and see if I can make it use getFoward(). I'd also prefer that, actually.

shartte commented 8 years ago

I amended the PR to use getForward() instead of getUp(). Luckily, all I needed to do was rotate the model around the X axis to make it work.

dpeter99 commented 8 years ago

Ok. But shouldn't we use the new naming for things ?

Elix-x commented 8 years ago

@dpeter99 it got auto formatted in https://github.com/AEModernMCPort/Applied-Energistics-2/commit/ad4580d34e9e2565c77c4f2a787f5d58b0de078a.