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

Model code changes #10

Closed dpeter99 closed 7 years ago

dpeter99 commented 8 years ago

So @elix-x , what do I need to do to make:

to work?

Elix-x commented 8 years ago

Stairs do have different super class (BlockStairs), but can't they be merged with simple block feature handler? Because slabs seem to be registered with block feature handler and not specific one.

Elix-x commented 8 years ago

@yueh, @dpeter99, we need to start to decide how cables should be partitionned (because i need them for model bakery testing :P). Cables will have 4 states on each side (no, part, block, dense), standing for things they are connected to. Depending on state, cable's model have to be extended differently. So we need to decide how many space each thing takes. Here's partitions i came up with: Cables

EDIT: :octocat: does not like my gif ;( http://i.imgur.com/PfMQd5m.gifv

yueh commented 8 years ago

Not sure what I am supposed to see, maybe it needs to spin faster?

But otherwise these cases apply to cables. Some of them might be passed to submodels/parts/whatever, but it aggregates it on a block level. (No guarantee to be complete)

Elix-x commented 8 years ago

I agree on most things, except for the fact that with models it kinda does not split in that way. It's 1.9, and we have conditional model multiparts (look at fence model in 1.9, and you will understand what i mean).

I would like to know dimensions (you gave them, but also some that aren't needed), so i'll start with cables: each cable needs "straight" center part, center part (1.7.10=cable without connections), small connection (1.7.10=connected to part), connection (1.7.10=cable connected to block/another 8ch cable) and extended connection (1.7.10=dense cable). But technically, we can remove small connection, as drawing cable's normal connection and then part will "hide" part of cable inside part and won't cause z-fighting. We can also remove extended connection, if we decide that when connecting to dense cables, cables visually connect in between 2 blocks. We should also not implement cable1-cable2 specific connections (except for ?-dense), as it will be too much even for code.

So, if we draw cables in following order and conditions, we will get ~looks like what cables looked like in 1.7.10:

  1. Center straight (log-kinda deal, draw looking to first side with connection).
  2. Center (if cable does not have straight connection) [will automatically "hide" center straight drawn previously]
  3. Normal connections (for all sides, when there are)
  4. Extended connections (for all sides connected to blocks without full connections / dense cables)
  5. Anchors&Facades
  6. Parts [will "hide" too long connections and parts of facades overlapping]
yueh commented 8 years ago

Conditional models will not cover everything. As you said, they support fences and can hide/show connections. But are probably not made to handle 20 or more combinations per side. Except you can load and merge them with others. Otherwise cables need to be fully extendable by addons. And I have no ideas how you would allow an addon to change the model files.

Just the center has 7 or 8 different states on how it connects/cable type and the same or more for outward connections.

For parts the cable must at least extend 1px short of the border. Some addons might just add parts, which are 6x6x1 large so the cable has to cover up 5px. Worst case even combine it from 5 1px thick cables or so. Like toggle buses or quartz fibers have the same size as a cable, so how about z-fighting (and no making it 0.0001px larger is not a solution). Quartz fibers are even transparent.

Maybe just handle a cable connection like a part? I would also recommend just look at them ingame. Better than explaining every case.

Do not forget that we also have to deal with the collision boxes. We need separate ones for parts, cables, the center etc. Is this supported by conditional models?

Elix-x commented 8 years ago

Models != server side. Collision boxes = server side

Condition models != conditional multiparts, though yes, they can't handle everything.

zFighting = idk how, but it does not happen. It works, so yay.

I'll see what i can do and look in 1.7.10 version.

yueh commented 8 years ago

Not the collision boxes in terms of player collision, but the ones used to show the current selected part when raytracing them (this is done client and serverside). I cannot remember it for certain, but they both should use the same bounding boxes. While fences just extend the whole box, we still need a multiple ones per cable.

Connections are at least static and cannot be changed by addons. But there are still 24 states per side or 48 with extending outside the block. Which certainly will break other mods as we will draw into their blocks or cables. If I recall correctly, there are even plans which at least double it again. And how would dense cables even connect, when do not render their own connection? They are not full blocks. So either they leave a gap, indication no connection. Or every single AE or addon or mod with AE support block has to be able to draw extended connection?

Handling nearly 300 permutations inside a json file just ask for bugs and is an extremely hard issue for humans to resolve. While computers are just built for these mundane tasks and do it without errors. Not saying that it cannot be exported to json, precomputing it from a simple definition during a gradle build would still be fine. It would just screw RPs. But at least not cause issue due to copy&paste the wrong stuff.

Facades also need to be translucent. Every one of them, not just transparent base blocks. There is an option to change them and at this point, cables will also need to be renderer through them.

Elix-x commented 8 years ago

Not the collision boxes in terms of player collision, but the ones used to show the current selected part when raytracing them (this is done client and serverside). I cannot remember it for certain, but they both should use the same bounding boxes. While fences just extend the whole box, we still need a multiple ones per cable.

Yep, and because they are both client and server side, models can't be used for them.

Connections are at least static and cannot be changed by addons. But there are still 24 states per side or 48 with extending outside the block. Which certainly will break other mods as we will draw into their blocks or cables. If I recall correctly, there are even plans which at least double it again. And how would dense cables even connect, when do not render their own connection? They are not full blocks. So either they leave a gap, indication no connection. Or every single AE or addon or mod with AE support block has to be able to draw extended connection?

Handling nearly 300 permutations inside a json file just ask for bugs and is an extremely hard issue for humans to resolve. While computers are just built for these mundane tasks and do it without errors. Not saying that it cannot be exported to json, precomputing it from a simple definition during a gradle build would still be fine. It would just screw RPs. But at least not cause issue due to copy&paste the wrong stuff.

I'll try to see if i can implement this with 1 or 2 cables, i may get it working...

No, it won't break RPs... Well, kinda. If during gradle build we generate and attach model files, they can still be overriden by resource packs. It's just that they don't have generation tools we do and will have to write them manually (models or their own tools to write models).

yueh commented 8 years ago

Yep, and because they are both client and server side, models can't be used for them.

No idea currently for a good solution. If we need to handle it internally and completely have to ignore RPs, I would say it is a valid option. It should just be consistent, either we allow RP to completely change parts or we prevent it. Not something like they can change cables (which are just parts) but not parts.

I'll try to see if i can implement this with 1 or 2 cables, i may get it working...

It is really just one cable and the types are just the one part, which can be attached to the center. Cable connections are essentially just the default parts, when a connection between neighbour blocks is established and only then. There is no canConnect() as connection just represent the internal network state. If they have a connection on this side, they query the neighbour for the connection type and render it. Otherwise they will not.

The connection type can even be upgraded to a higher type depending on the neighbour. Like a glass cable will render a covered connection, when attached to most AE network blocks. This can go up to dense connections. For example it was an idea to render every connection to a controller as smart connection. This is not used in AE currently, but any addon could still make use of it.

Elix-x commented 8 years ago

No idea currently for a good solution. If we need to handle it internally and completely have to ignore RPs, I would say it is a valid option. It should just be consistent, either we allow RP to completely change parts or we prevent it. Not something like they can change cables (which are just parts) but not parts.

Sizes will have to be hardcoded. Because what whould happen if 2 players are using different RPs and size in each are different?

It is really just one cable and the types are just the one part, which can be attached to the center. Cable connections are essentially just the default parts, when a connection between neighbour blocks is established and only then. There is no canConnect() as connection just represent the internal network state. If they have a connection on this side, they query the neighbour for the connection type and render it. Otherwise they will not.

The connection type can even be upgraded to a higher type depending on the neighbour. Like a glass cable will render a covered connection, when attached to most AE network blocks. This can go up to dense connections. For example it was an idea to render every connection to a controller as smart connection. This is not used in AE currently, but any addon could still make use of it.

I was looking into it currently :wink: ...

Elix-x commented 8 years ago

Cablez

Elix-x commented 8 years ago

Nyan Cablez

Elix-x commented 8 years ago

@yueh Why is AECableType a enum? Are we like "nah" to adding more cables? Or we expect addons to master EnumHelper?

Also, i'll stop spoiling cables for now...

yueh commented 8 years ago

On a technical side there are no cables in AE2, thus the AECableType is (or should) be mostly used for clientside rendering and how a existing connection should be rendered on the side of AE2. It might be an idea to refactor it and use @SideOnly, if applicable. But then @SideOnly is more or less broken without any fix in sight.

So yes, there is not intention to allow more cables. It would also conflict with your approach of using the vanilla json multiparts to render the cables instead of using a actual multipart system and treat cables just like any other part.

Elix-x commented 8 years ago

So yes, there is not intention to allow more cables. It would also conflict with your approach of using the vanilla json multiparts to render the cables instead of using a actual multipart system and treat cables just like any other part.

Well, i 50% left it. I combined 2 systems to allow parts to specify dynamic models (like depending on their state). So conflicting aside, should it still be an enum?

yueh commented 8 years ago

I do not see any better implementation for java currently. The alternatives would be pretty much something like return a Class<? extends CableType> and do stupid instanceof checks or use an int or String and have a full blown cable type registry, which nobody ever will use. (And also break the rendering)

The only sane alternative in Java would probably be use real polymorphism and use soemthing like a strategy, which then handles the submodel generation based on the data it receives. But that is the exact opposite of what you are currently doing.

The only real alternative to an Enum, would be to move to scala and use a sealed trait. Internally it would still use instanceof and such things, but that is hidden behind the abstraction. So you still have to only deal with what you want to achieve and not how you do it.

Elix-x commented 8 years ago

Well, in vanilla items are singletons which are held by itemstacks, which are individual. Couldn't we do the same thing? Singleton cable types holding information that is the same accross individual IPart instances.

yueh commented 8 years ago

Vanilla is not using singletons. (I am not even aware of any singleton in minecraft, maybe except Minecraft)

It would pretty much like replace java enums with your own custom implementation because of 'not-invented-here-syndrome'. For something which just indicates 'if AE has a connection to this tile, render it as X'.

Elix-x commented 8 years ago

Vanilla is not using singletons. (I am not even aware of any singleton in minecraft, maybe except Minecraft)

It would pretty much like replace java enums with your own custom implementation because of 'not-invented-here-syndrome'. For something which just indicates 'if AE has a connection to this tile, render it as X'.

Well, why not? After all, if they implement their own cable, they take care of rendering it and it's connections, not us.

yueh commented 8 years ago

As said, it is pretty much only used for AE itself. It is completely optional for anyone making their own cables, but if they ever connect to any normal AE cable, we need this information.

It will also only ever be limited to the cable types we support. Nothing else. So allowing any arbitrary subclass instead of a well defined enum will just cause issues.

Elix-x commented 8 years ago

Well, we can make the class final and hold all instances. So it's not limited to what we need, but it also does not allow to extend class and mess with stuff.

yueh commented 8 years ago

Why do you want to replace an enum with your own custom enum? Just to write class and not enum? Enum is just a final class, which holds all subclasses of it and an instance of them. But with all the advantages of having a special treatment by the compiler. Like use it for switch.

Just to be sure, could you please explain what you think it does? Because currently I not certain, that you understand its intention completely. Currently it sounds really like "I just want to replace it, but I can not even list what advantages it would have or even how."

Elix-x commented 8 years ago

Well, previously, it was just acting as a type of AE-vanilla connection to render. With model's system nature, i had to refactor it, because we need to map cable models' used textures to texture atlas while stitching, so we need to load models, get all textures from model and map them. I agree, enum is very useful for that. Now let's say, somebody wants to add new cable type and render connections to his tes with his cable type. If we leave it as enum, in the best case he will use EnumHelper.

yueh commented 8 years ago

There is no "somebody wants to add new cable type". It will simply never happen.

Elix-x commented 8 years ago

Why? Anything can happen?

yueh commented 8 years ago

It is nothing we have to care about it. We simply connect as our cable type to any block and that is it. There is no need for trying to extend EnderIO cables into our blockspace. It will look horrible due to their alignment and why invest the time to let it look horrible?

If someone copies our cables and say make light emitting ones (that is as far as I remember actually a feature request), then it is still a glass, covered, smart or dense cable on our side. Not some light emitting one.

Elix-x commented 8 years ago

Ok i'm leaving enum as it is... They have EnumHelper after all...

Meanwhile cables are working, but @dpeter99 will have fun times manually playing with UVs, as currently not everything is aligned properly (channels on smart & dense cables).

dpeter99 commented 8 years ago

OK. it sounds "fun" I will look at them later this day (UTC+2)

Elix-x commented 8 years ago

Well, sorry, not today. I'm not ready to commit cables & parts stuff yet. Should be done with them soon, though.

yueh commented 8 years ago

There is also ReflectionHelper or whatever to remove any final from a class. Or ASM, whatever. If someone wants to be an idiot and tries to manipulate the bytecode, there is really nothing we can prevent.

An easy fix would also to just allow our own cables or nothing as center part. Would there be an official forge multipart API, it would be a bit different. But for our own system, there is no need to support every possible multipart use case. Just allow mods to add outer parts and have AE handle the center or plain cable connections would be sufficient for like 99.99% of the possible mods. Whoever wants to add cables which have a 8x8x8 center, they still have to implement their own system including as it would already clip with most of the normal parts.

Elix-x commented 8 years ago

Well, if they implement new cables, they take care of them.

Meanwhile, why is there NONE cable type? Can't we simply return null when there's no cable?

yueh commented 8 years ago

NONE means render nothing, not there is no connection. And you should know my opinion about "simply return null" :wink:

Probably never be used or at least extremely rare, but I can see one or two uses case.

Elix-x commented 8 years ago

Well, yes, i know that already. But in this case, there's no cable, there's nothing. Kinda logical...

Only rare case could be returning it from tile that does not want to render connection, but if you don't want to render connection, there's no connection / empty connection / null connection...

Right now, if TE returns NONE, it will cause all cable to go white-purple box... Not ideal at all.

yueh commented 8 years ago

You could potentially add something like a directional wireless emitter injecting a connection directly into cable or tile. Say to have a free floating block in the middle of your base and hide the cables. In a rare case you could place it next to a cable and face that, by default the cable should then render a connection, but that is clearly wrong in this case.

Or say someone really wants to add their own cables, they could then check of there is a cable next to them. If so, report NONE and extend their own cable to fit correctly.

Elix-x commented 8 years ago

Or say someone really wants to add their own cables, they could then check of there is a cable next to them. If so, report NONE and extend their own cable to fit correctly.

Very ideal implementation. Return NONE, cause all neighbour cables & tes to become black&purple boxes / not render connection and render everything for them. Totally implementing it.

yueh commented 8 years ago

If our own cables are unable to render no connection regardless of having a neighbour or not, that implentation is already flawed.

"No connection" is a simple requirement for no connection due to a non grid TE, no connection possible on this side etc. If adding another condition for it like the neighbour reporting NONE breaks it, there are already more issues with the rendering implementation.

Elix-x commented 8 years ago

They are... I was just messing with commenting out some parts of code (like if(connection!=AECableType.NONE){).

yueh commented 8 years ago

Having something like getCableConnectionType().bakeModel(data) would probably be a more clean approach in terms of software design. But AECableType is a part of the API and we cannot really ship internal code with it. Some addons will ship it and the forge random classloader can potentially use an outdated class breaking whatever. So at the API level we pretty much have to resort to interfaces, enums and (interfaces to) factories.

The actual AECableType for a specific side of the current block is more or less maxCableType(this.getCableConnectionType(), neighbour.getCableConnectionType()). So why not something like

cableType = this.getActualCableConnection();
switch (type) {
   case NONE: return cableGenerator.bakeNoConnection(data); break;
   case GLASS: return cableGenerator.bakeGlassConnection(data); break;
   case COVERED: return cableGenerator.bakeCoveredConnection(data); break;
   ....
}

AECableType is pretty much the same as any blockstate property a specific side would have. With NONE basically handling

The last one might be tricky. But maybe inverse how the missing cable on parts is rendered. Do not try to guess the length, just let the part handle the missing 1-3 px long cable. They know how much they need to complete it. So just have a utility class for creating any cable connection with a specific length, which can either be used by the cable or part model baker.

Elix-x commented 8 years ago

That's also one of the reasons i wanted it to be a non-enum: to not ship baking things with API. So we create all cable types with rendering methods internally, and API users can access them same way they can access AE's items.

yueh commented 8 years ago

I still do not see any advantage of exposing internal methods/functionality to every mod in a way they can easily break by passing around their own implementation.

AECableType is just data our own render will respect and act accordingly. Like a dense cable rendering a dense connection to a tile supporting 32 channels like the QNB. Nothing more.

Converting it an interface with a factory exposing the internal classes, will just lead to idiots seeing the interface, implementing it and passing it around. Because reading the documentation is way too hard compared to making their own implementation. An Enum at least needs to intentionally be broken. Interfaces are by design there to be implemented, but not to break on implementation. Or you end up in situations like return new appeng.internal.SmartCableConnection(); and not using the singleton provided by the API.

I personally do not see any reasons to expose anything of the renderer implementation except the data it uses. So no exposing of helpers to render cable connection or whatever. Addons just expose things like AECableType or their model for part including the dimensions of it and that was it. If some really want their own cables it is pretty much unlikely that they can use our renderer for it as it will certainly not fit their design.

The only requirements for the cable render in terms of addons is, that it can use their part models and bake it together with our parts, cables and potentially missing parts. Not need to have a full blown multipart/cable rendering library. Think of a PartModelGenerator wrapper. Getting passed the (addon) part, cable type, using the dimensions and model provided by the part, adding the missing cable connection and pass it back to the CableModelGenerator, which then rotates it around to fit their correct side.

Elix-x commented 8 years ago

Ok, let me finish current part system and then we will look at how it can be improved changed. Right now, it's already working, dynamic and extendable. And i'm done with cables & facades.

yueh commented 8 years ago

It is fine when it can support it. It should just be careful about what it exposes at first.

Elix-x commented 8 years ago

It should just be careful about what it exposes at first.

Same rendering methods it didin 1.7.10, but only with different params...