Minecraftforge: [Enhancment] Entity Definitions

[jredfox]

why?: no set way to identify if an entity is a specific type without checking spawn types which is how they spawn not necessary what the entity is especially if you use JAS mod inn 1.7.10+

Edit: I have created a basic system for types. Note this is very basic implementation the registry system would have to be called manually instead of in the constructor it's just an example of what should be done for having definitions of both info and entity types:
https://gist.github.com/jredfox/4b14e86c0ba1dd204f33f294dc1987e6

[MajorTuvok]

I'd say that using a boolean for all these different things would overcomplicate matters. Also It would prevent mods from easily adding their own type (yes you can add it as a ResourceLocation, but it's just as if you're mixing 2 different api styles which would confuse quite a few people).

So maybe combine all of these in a Set< ResourceLocation> or a Set< IEntityAffinity > where IEntityAffinity is an interface who's instances should be singelton's to allow for fast reference checks. We could also add properties like an unlocalized name f. e. ...?

[jredfox]

I agree however these are vanilla types you need to quickly without looking at strings is this this type without strings if necessary. Maybe an enum would be good for vanilla And mods could use resource location for modded type or mods add their own enums with a .getName() which will display a resource location of a name.

However resource location might not be applicable as many mods might want to just be a "knight" and your telling me they want them all separate? I think it should be optional to put in the modid as key.

Also it should return a Set[EnumMobType] since mods could have more then one type

Edit just updated the enhancement but, it was a general idea I shouldn't have to write an almost full pull request for people getting the idea and issue here.

[MajorTuvok]

You cannot use an Enum for modded ones (customizability...) ...
Because I disliked the idea of using ResouceLoctions (I only ever mentioned that because you had that in your original proposal, so don't yell at me - it was your idea) I added the Interface approach...
Next I already said that one should return a Set of (like you are calling it) IMobType.

You could make the Vanilla-Enum implement the Interface and then combine them in one Set. This would have the benefit of both being combined in one Set (making it both more readable and easier to use) ...

Edit:
I will try to explain more clearly what I mean tomorrow or on sunday...

[jredfox]

Edit: it's actually possible but, it would be a pain switching to objects.

Updated commit to match an object version of this. Using simple resource locations like I said before would cause more issues then it would solve.For example two mods want the same type as fox well now they need to say the exact same domain in order for it to be compatible it would kind of be stupid.

My recommendation is objects that way people can extend them and store more methods. Interfaces are ok but, almost no mod would use them properly for example vanilla minecraft doesn't use it's own for all projectiles. If you store it as a method people I feel would use it more.

Code:
https://gist.github.com/jredfox/4b14e86c0ba1dd204f33f294dc1987e6

[jabelar]

What is the purpose of the type? Is it just for where it spawns? Because I just wrote a similar response to the other issue (about water breathing) -- it is difficult to figure out a way to group the entities that will server multiple purposes.

For example, the Java type hierarchy for entity classes already groups them as living versus player, as creatures versus players, as ambient creature versus creature versus dragon versus flying versus slime versus watermob, and so on. These types are mostly grouped by behavior since in Java the child is supposed to inherit the parent behavior and then further modify it.

The existing Java type hierarchy is obviously useful for some logic, but doesn't help that much with spawning, or with knowing which entities are most dangerous, or which ones drop the best loot, or whatever other type of grouping you might want to list.

When I look at your suggested types in your gist link, it seems confusing to me -- you have "water" and "boss" as separate types. What if I want a "water boss"? I think you are proposing to allow multiple tags but it still isn't clear which ones would be mutually exclusive -- could I have an entity that is tagged with ALL of the types?

If you really want to do it I think you need several lists of types for different purposes. For example you might have spawn type, movement type, breathing type, combat type, tameable type, difficulty type, and so on. So then something like the dragon would have spawn type = end, movement type = flying, breathing type = air, combat type = fireball, tameable type = no, difficulty = final boss, or something like that. And a sheep might have spawn type = normal, movement type = land, breathing type = air, combat type = passive, tameable type = no, difficulty = normal.

Hope you get my point. Need multiple logical sets of types for each purpose you might use types for.

[jredfox]

the purpose of type is to define what entity type it is is it a water type fire etc. nothing to do with spawning if you read the initial comment that would have answered question one. Currently there is no set way in checking an actual entity type only checking spawns which can be 100% wrong. For example you can check an entity's spawn type and it returns water for pterodactyl well in reality it is a flying creature might be considered a water creature based on it's spawn type when in reality it is solely a flying creature. Also checking things like fire and water have booleans but, they are unreliable just because a mob is fireproof doesn't mean it's a fire mob same with water.

yes mods would have to implement things(including vanilla implementation) to be compatible with mods checking the entity types. That is the entire point of the issue not knowing what type an actual entity object is I don't care about spawn type I want actual entity type. The EnumCreatureType by vanilla is made for only spawning the entity so using it as a boolean for everything is wrong and should be renamed to EnumCreatureSpawn As more proper definitions like the code I posted above would solve. I am not going to write the full implementations as this isn't a pull request.

Again there would be no object oriented issue since mods would implement their own entity types list and my system has a getPrimaryType() as well as a list of applicable types for the entity. The isLiving type for info is to simply state for modders is this creature a living entity even though it doesn't extend said class and is possible. You wouldn't cast the object to EntityLiving it's just simply an info for mods to scan say while using the /butcher command so the command knows whether or not to kill said entity

So here is why you need entity types here is a crappy hard coded butcher command because there is no proper definitions:
https://github.com/jredfox/lanessentials/blob/master/src/main/java/com/EvilNotch/lanessentials/commands/CommandButcher.java

[diesieben07]

You use the term "fire mob". What exactly constitutes a "fire mob"? Who decides?

[jredfox]

the entity decides if it's type fire mob in the arraylist of types. Also can decide if it's the primary type

[diesieben07]

Unification and standards don't mean anything if you don't actually define what they mean.

[jabelar]

For example you can check an entity's spawn type and it returns water for pterodactyl well in reality it is a flying creature might be considered a water creature based on it's spawn type when in reality it is solely a flying creature.

That's my point. You need to have a set of possible "types" defined for each category of functionality that you might care about. For this case I would propose your pterodactyl would have spawn type = water (indicating it spawns in water biomes), breathing type = air, movement type = flying, and so forth. It would be very clear what each type means in each functional context.

[MajorTuvok]

I'd agree with jabelar that there might be a need of multiple funcionalities, though I think some/all types should allow multiple values. (If that was meant with your example I'm sry for misunderstanding you Jabelar)
For example something might have multiple combat types.

Maybe this and Jabelars PR should be combined?

[jredfox]

my opinion is three like I have been saying:
one for spawning: EnumCreatureTypes is already in place just rename the enum to "EnumCreatureSpawn"
one for info: is this creature living non living, projectile, etc...
one for actual type like fire,water ArrayList also a primary type for modders to say hey this entity primary type is flying.

There should also be a method for entity.containsType(EntityType) entity.containsInfo(EntityInfo) so people don't constantly manually iterate and then blame forge rather then their mod.

I actually wrote both classes for the other types here as well as the methods to be implemented by the entity.class and all other entities that need to override the said methods to be properly implemented:
https://gist.github.com/jredfox/4b14e86c0ba1dd204f33f294dc1987e6

[MajorTuvok]

Seems mostly ok to me, although I'm just asking myself why someone would create his/her own EntityLiving hierarchie and thereby make himself incompatible with almoste the complete Minecraft codebase...

What about the movement types and combat types? (I presume you haven't included them for simplicity)

[jredfox]

@MajorTuvok it wouldn't be incompatible unless they manually check is this entity instanceof EntityLiving if that's the case then yes forge would have to patch alot of classes to entity.containsInfo(EntityDefintions.living) rather then simple if statement checking the instanceof the class.

people would make their own entity living version as the current setup might not work with their entity or they might not need all the additional data that is generated in entity living base.

As for EntityType I have no idea why they haven't done this before it's a couple classes to patch in the entity package and call it quits maybe some addition patching in methods where it checks instanceof water creature and stuff like that but, that's about it

EntityInfo also contains info for is hostile and is peaceful as those need checks to although if they are not going to use the rest could technically be merged with entity types which I wouldn't recommend

Edit: I am going to try to edit the gist look again it should be good now.

[MajorTuvok]

What I meant is that a lot of mods use instancoof checks and while it is (comparative) easy to patch these usages, mods will not do this so easily and rather start complaining about having to sort ojt all these kind of checks

[MajorTuvok]

Though the EntityType will not create any problems in my eyes and is needed for sooo many things.

[jredfox]

right so this update will change the way mods check things that's the point proper definitions

[MajorTuvok]

What about tagging entities?
As of MC - 1.13 this seems to be the consistent way for me...?
For reference: MinecraftForge/MinecraftForge/issues/4577
If the oredict is removed in favor of tags, then why don't we tag entities too? (might very well already be possible in 1.13, in this case forge would only need to provide some common categories...)

[jredfox]

I think this is cleaner and more user freindly

[MajorTuvok]

I'm actually not sure about that, f. e. tags can be declared in jsons if I recall correctly...

[MajorTuvok]

Ok I just looked it up according to the minecraft wiki (https://minecraft.gamepedia.com/Tag) Tags don't apply to entities in vanilla Minecraft 1.13 - so this would have to be added...
It would actually be quite convenient, being able to use entity tags in commands, etc...
@jabelar @diesieben07 @mezz, what do you guys think?

[jredfox]

you shouldn't be able to config entity types it's the coders intention so jsons would be a no no

[MajorTuvok]

I didn't mean that users should edit them - I meant that modders could define them in jsons

[diesieben07]

I didn't mean that users should edit them

Why not? I can see being able to define a group of entity types and acting on all entities in that group at once being quite useful for server owners.

[MajorTuvok]

Oh that's true...
I always only think of singleplayer games where I don't do that much of configuration effort

[jredfox]

because then you could say this entity type of fire = water and screws thing up and crashes for other mods. especially if it's the primary type

[SkyHawkB]

Hmm... I think if you wanted to have entity types you should just code it yourself, in the mod that needs it... or make a library.

[MajorTuvok]

Fox, you could only add tags, not remove them (as far as I know). And if some Modpack author wants to do that, he may do that and see how the mods react to both a fire and water entity (which should be possible in any way and would be with your proposal too). I can very well think of an both water and fire entity.

[jredfox]

"Hmm... I think if you wanted to have entity types you should just code it yourself, in the mod that needs it... or make a library."

I did code it myself. Would be uterally useless if only my entity classes used them it has to be forced into the system thus the mod owner is responsible for both types and info not me as it's impossible otherwise

[SkyHawkB]

Well, you technically could modify Minecraft's classes, and add compatibility with other mods later. Or code it in a way that doesn't require modifying classes.

[SkyHawkB]

Forge isn't going to patch Minecraft just so that one person's mod can work better, and force other mods to use a system only one mod was made for.

[jredfox]

Has nothing to do with me it's proper definitions for everybody you would know if you just read a quarter of the comments and actually tried to code yourself to see what entity is what.

I am done commenting here as it's pointless the code is written forge will write a pull request itself as it's not just me but, practically everyone that needs this thinking it's only my mod is wrong I actually don't even have any code for this yet but, need to know what entity does what later with proper definitions as well as alot of people modding forge. And if forge refuses do it it out of ignorance it's not my problem that they need to learn proper coding edicate has nothing to do with the person who brought the issue to the light nor my coding abilities has to do with the actual issue of no definitions to properly determine entity type and info as there is no set definition.

[covers1624]

What? You want a feature. Have a mock up of the feature. And want someone else to write the PR? No that's not how this works. YOU want the feature, YOU PR it.

[jredfox]

not just me that wants it 3 people here countless of others. I am to implement a whole bunch of code for forge. This isn't a self fix program do it yourself you create an issue it's not your job to fix said issue.

[covers1624]

I have contributed a few fixes and features, you obviously don't understand the work flow for open source projects like Forge, they aren't a company, they don't have large chunks of funding to hire people to handle issues and features, everyone who works on forge does it for free, because they love extending Minecraft, forge does not have the man power for such a low priority feature such as this. So if you want the feature pr it.

[mezz]

It's not our responsibility to implement your features, or even to fix "critical bugs" you report. Personally, I spend my time on the project because I want to help people contribute so that mods work together and ultimately so that Modded Minecraft is fun.
This is not the first issue you have created that has ended in this kind of reaction, it seems very clear to me that you do not really care to understand what your own responsibilities are. You are acting in a stubborn and entitled way, and trying to make other people do work for you. Please either figure it out and contribute, or pay me a very generous salary, or stop submitting issues here.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.