Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop 'Entity' suffixes #407

Closed
Runemoro opened this issue Jan 16, 2019 · 44 comments
Closed

Drop 'Entity' suffixes #407

Runemoro opened this issue Jan 16, 2019 · 44 comments
Labels
discussion vote A vote on a name refactor

Comments

@Runemoro
Copy link
Contributor

Runemoro commented Jan 16, 2019

  • There's no ambiguity in almost all cases
  • It would make code less verbose
  • We don't say "five cow entities", but rather "five cows" (but we do say "five stone blocks" rather than "five stones", which is why those should keep the Block suffix)
  • Mojang doesn't use any Entity suffixes
@Runemoro Runemoro changed the title Drop 'Entity' suffix Drop 'Entity' suffixes Jan 16, 2019
@LemmaEOF
Copy link

This is just dropping Entity for regular entities and not BlockEntities, yeah?

@Runemoro
Copy link
Contributor Author

Yeah.

@liach
Copy link
Contributor

liach commented Jan 16, 2019

Sponge does not use Entity suffixes as well.

@EduardoRFS
Copy link

EduardoRFS commented Jan 28, 2019

+1 but can't that argument also be used to things like blocks? It almost never clash. I like the idea of blocks. Chest but if you're making something that isn't strictly block related, it will be strange.

@Prospector
Copy link
Contributor

I disagree for consistency's sake

@Sorixelle
Copy link

The thing with the Block suffix, IMO, is that it denotes not just a block, but the block. Take grass for example, there's only ever one instance of a GrassBlock at any given time, so it's safe to say "this is the grass block". Entities don't really work like this, because there's an instance of the entity for each one in the world. So, as another example, with zombies, you can't say "this is the zombie", because there are multiple zombies, not just one. You'd say "this is a zombie".

On the other hand, the consistency argument makes a good case - it's a bit inconsistent if we suffix some things with their type and leave the others with no suffixes.

@liach
Copy link
Contributor

liach commented Jan 28, 2019

hmm, how about falling block?

@EduardoRFS
Copy link

@Sorixelle makes a really good point. But i think that we should find a reason to the suffix, yeah it's cool to not have on the name, but isn't consistent.

@liach
Copy link
Contributor

liach commented Jan 28, 2019

so ZombieEntity means the Entity type for zombies?

@Runemoro
Copy link
Contributor Author

Runemoro commented Feb 12, 2019

can't that argument also be used to things like blocks?

I disagree for consistency's sake

The difference is that we say "I have 5 stone blocks" (not "5 stone"), but "I have 5 sheep" (not "sheep entities").

@asiekierka
Copy link
Contributor

If you say "I have 5 stone" please 👍 this issue

@Runemoro
Copy link
Contributor Author

Imagine if we named the classes SheepThing and ItemThing.

That's exactly what we have, because "entity" is just a synonym for "thing" and "object".

@JamiesWhiteShirt
Copy link
Contributor

Is "entity" a synonym for "thing" and "object"? Yes.

Does that mean that entity cannot be something more specific in Minecraft? No.

@Runemoro
Copy link
Contributor Author

Entities encompass all dynamic, moving objects throughout the Minecraft world.

Would you be OK with the names SheepDynamicObject or SheepGameObject then?

@JamiesWhiteShirt
Copy link
Contributor

"article", "object" and "thing" are synonyms of "item". Should we rename SwordItem to SwordArticle, SwordObject or SwordThing?

The point is that SheepThing and ItemThing are meaningless variations when the concept of an "entity" is well defined as something more specific than a "thing" or an "object" in Minecraft, just like "item".

@Runemoro
Copy link
Contributor Author

Runemoro commented Nov 22, 2019

I'm not saying that Entity is meaningless. I'm saying that it's a general term that's used for all dynamic things, the same way we have java.lang.Object, but not StringObject, IntegerObject, etc.

The point I was trying to make by that comparison was that if Mojang had named Entity Thing instead, would you still be for having SheepThing, ItemThing?

@i509VCB
Copy link
Contributor

i509VCB commented Nov 23, 2019

"article", "object" and "thing" are synonyms of "item". Should we rename SwordItem to SwordArticle, SwordObject or SwordThing?

The point is that SheepThing and ItemThing are meaningless variations when the concept of an "entity" is well defined as something more specific than a "thing" or an "object" in Minecraft, just like "item".

I don't see a reason for making it more complex with item names into article or thing.

Shulkers are great examples of where removing the entity denotation can make things confusing due to similar naming scenario and a block entity, block and entity definition of same name.

@haykam821
Copy link
Contributor

I'm not saying that Entity is meaningless. I'm saying that it's a general term that's used for all dynamic things, the same way we have java.lang.Object, but not StringObject, IntegerObject, etc.

java.lang.Object is a superclass of every class, and entity classes make up a fraction of that.

@Linguardium
Copy link

how would you separate Item from ItemEntity, for example.
Also, Java standard shows EnumSet, EnumMap etc, so base class is part of extenders
CompoundTag, ListTag, StringTag...

Adding the super type is common practice

@liach
Copy link
Contributor

liach commented Jul 28, 2020

Drop for the entity for ItemType for the registry element

@haykam821
Copy link
Contributor

The minecraft:item registry is a Registry of Item, not ItemType, and the entity type ID is minecraft:item so it is ItemEntity.

@liach
Copy link
Contributor

liach commented Jul 28, 2020

Sure, keeping Entity suffix is no big deal. We can live with that.

@YanisBft
Copy link
Contributor

Yeah, don't drop Entity suffix please. It will only lead to confusion, and instead of naming things accurately, we will just try to name things not to be confused with suffix-less entities

@Earthcomputer
Copy link
Contributor

The point I was trying to make by that comparison was that if Mojang had named Entity Thing instead, would you still be for having SheepThing, ItemThing?

Yes, because Thing would have a specific meaning in that case.

Like Yanis said, dropping the suffix only leads to confusion. Names are for clarity, not "looking good".

@Runemoro
Copy link
Contributor Author

Names are for clarity, not "looking good".

Names sounding natural is just as important as clarity.

Imagine I said while playing Minecraft "I'm going to kill some cow entities to get some raw beef items and then use a furnace block to turn them into cooked beef items".

Code matching the way we think and speak is a very good thing.

@haykam821
Copy link
Contributor

haykam821 commented Aug 11, 2020

You can have both at the same time: a natural base (cow, raw beef, furnace), and a clarifying suffix (entity, item, block).

@Runemoro
Copy link
Contributor Author

The point is that the suffixes are unnecessary. We know cows are entities and beef is an item. No one says "cow entity" or "beef item", so why should we have to say/read that when writing/reading code?

@LemmaEOF
Copy link

there's still the problem of ItemEntity.

@i509VCB
Copy link
Contributor

i509VCB commented Aug 11, 2020

Yeah Item is the sole reason I don't see this working. I probably wouldn't agree to it unless we did Item -> ItemType but that is an even bigger rename due to the shear amount of items in the game.

The other option is the clarify when names are ambiguous but that causes names to be inconsistent.

@Runemoro
Copy link
Contributor Author

ItemEntity -> DroppedItem, similar to the way we have ThrownSnowballEntity

@haykam821
Copy link
Contributor

We know cows are entities and beef is an item. No one says "cow entity" or "beef item", so why should we have to say/read that when writing/reading code?

The only other option is to be inconsistent no matter the solution.

Dropping the suffix would cause issues with classes for content from different registries. The Item base class is definitively named that way because there exists a registry full of it (or its subclasses) using the identifier minecraft:item. Likewise, the ItemEntity entity class is definitively named that way because its respective entity type is registered in the minecraft:entity_type registry as minecraft:item. The current Item class would stay as Item, but the ItemEntity class would also become Item.

If this specific issue was resolved by changing the name of the latter class to DroppedItem or similar, then it would no longer match the identifier. This is a problem as there is enough faith in the identifier being a good representation for the class name that we have the current automatic system of naming fields in registry content classes, and adding the registry suffix to the class name.

The entire idea of dropping the suffixes is inconsistent with Java standard as well (see implementations of List or Map), as previously mentioned.

I probably wouldn't agree to it unless we did Item -> ItemType but that is an even bigger rename due to the shear amount of items in the game.

I'm sure there will be an ItemType as soon as Mojang makes items data-driven, so it's best to hold off on this for now. Plus, this would still be inconsistent with the identifier and our current conventions regarding minecraft:x_type registries.

ItemEntity -> DroppedItem, similar to the way we have ThrownSnowballEntity

Once again, this would be inconsistent with Java standards and Yarn standards. I'm curious about any discussion regarding ThrownSnowballEntity though considering its identifier is minecraft:snowball.

@Runemoro
Copy link
Contributor Author

Runemoro commented Aug 11, 2020

The entire idea of dropping the suffixes is inconsistent with Java standard as well (see implementations of List or Map),

A HashMap isn't a hash, but rather a hash[-based] map, and an ArrayList is not an array, but rather an array[-based] list. A CowEntity is a cow. It's a bit like saying we should say "cow animal" rather than "cow" because we say "pencil sharpener" rather than "pencil"...

Look at Java AWT. Everything extends Component, but we just have Button rather than ButtonComponent, Scrollbar rather than ScrollbarComponent, etc. So if we follow the Java standard, we should get rid of suffixes.

If this specific issue was resolved by changing the name of the latter class to DroppedItem or similar, then it would no longer match the identifier.

I agree that matching the identifiers is important to make classes easy to find, but I don't see why we have to match it completely. Adding some clarifying words such as Dropped is ok.

Between "match all the identifiers perfectly to satisfy my OCD" and "have names that are nice to use", I'd choose the second.

I'm sure there will be an ItemType as soon as Mojang makes items data-driven, so it's best to hold off on this for now

I think Item -> ItemType (as well as Block -> BlockType) is very important and should be done as soon as possible. Item is just wrong, since there isn't an instance of the class for every item. It even leads to people new to modding to make the mistake of storing the item's state in fields in the item class.

inconsistent with the identifier and our current conventions regarding minecraft:x_type registries.

That convention doesn't make sense. The registries store item types and block types, not the instances of each individual item and block. The variation on the _type suffixes is an inconsistency by Mojang, and we shouldn't copy it, just like we wouldn't copy a typo in an identifier.

@Earthcomputer
Copy link
Contributor

Earthcomputer commented Aug 11, 2020

To be honest Rune, "I'm going out to kill some cow entities" is not far from sounding natural to me, coming from the tech community.

If you want to make Minecraft read like natural language, there's a lot of other renames you can do. "I'm going to chop down some tree features" doesn't read well for anyone. Does that mean we should rename TreeFeature to Tree? No! But "cow entity" actually reads well. Especially in a context like "the server is lagging because there are a lot of cow entities in one place".

@Runemoro
Copy link
Contributor Author

Runemoro commented Aug 11, 2020

Does that mean we should rename TreeFeature to Tree?

Yes, the word Feature here is redundant here. I'd also suggest adding Generator at the end of the feature classes, since they're actually generators for the features, not the features themselves. We'd have something like TreeGenerator extends FeatureGenerator.

the server is lagging because there are a lot of cow entities in one place

I'm guessing the reason you'd word it like this is to put emphasis on the fact that it's entities, which happen to be cows, that are lagging the server.

But in general this emphasis isn't necessary. If I'm working on a regular mod that just adds content (not a performance improvement mod), I'd be thinking about "spawning some cows", not "spawning entities of the type cow", and I'd want to write my code that way.

@l-Luna
Copy link

l-Luna commented Aug 11, 2020

But they are Cow Entities, not Cows. The idea of a cow only exists in natural language - CowEntity specifically refers to the cow entity, not the entire concept of a cow. You wouldn't put code that relates to other parts of the cow - such as its drops - in that class.

@LambdAurora
Copy link
Contributor

LambdAurora commented Aug 11, 2020

This bikeshedding is just useless...

The Entity suffix is clear as it doesn't let you doubt over "what is this class?". In the context of Minecraft Entity is clearly not a synonym of Thing.

Also for fucking modders sake, don't introduce a breaking change this big, it's already complicated enough when updates come you don't have to make it harder for modders by fucking their workspaces because someone disliked the Entity suffix.

Like sure, go ahead change it, and watch the whole world burn because now mods don't compile because of 500 errors. Sure there's migrateMappings but why would I have to run it for a change so meaningless.

This issue has been opened for 1 year, and it was not applied. And meanwhile people used the Entity suffix. Personally I would be very pissed to see such a change being merged thus exploding all my current mod workspaces.

Also, ok natural language is cool and all, but this is specific to the game engine, trying to read code like a novel is just not possible.

@bluebear94
Copy link
Contributor

I'm going to have to say no, too. There are a lot of cases when it would be ambiguous (the aforementioned ItemEntity, as well as ArrowEntity or TridentEntity).

@modmuss50
Copy link
Member

Im not a fan of this idea, I dont think it really boils down to how it said in spoken language. It should be consistant across yarn, and removing it from just entities seems odd, removing it everywhere (blocks and items) doesnt make any sense at all.

@liach liach added the vote A vote on a name refactor label Aug 11, 2020
@liach
Copy link
Contributor

liach commented Aug 11, 2020

Looking at the vote, guess it's clear?

@TheBrokenRail
Copy link
Contributor

TheBrokenRail commented Aug 11, 2020

Do you call LivingEntity: Living or Entity: nothing, the suffix provides consistency with other names, I agree @l-Luna with CowEntity does not represent a Cow it represents a CowEntity, a Cow represents the entity, its drops, and its model/render-er. The Entity suffixes are aren't hurting anybody, and I think they help add some clarification just in case, along with the fact that BlockEntity is a used suffix as well. Just my 2 cents.

@Runemoro
Copy link
Contributor Author

Do you call LivingEntity: Living or Entity:

I call LivingEntity a "living entity", so that name should stay. I call CowEntity a "cow", so that name should be changed.

@LambdAurora
Copy link
Contributor

LambdAurora commented Aug 12, 2020

My problem is you are trying to name things like it was server-side only, sure naming CowEntity Cow on a server-side only API would make sense as it's literally the entire concept of cow for a server.

But here there is also the client, naming CowEntity Cow is plain wrong as the cow is defined by it's Entity which is the object handling the properties of the cow, how it interacts with the world and its goals; but also by its model and its renderer!

By naming CowEntity Cow you strip a big part of the meaning of the class and make it then misleading.

I would be fine with this if yarn was server-side only, but it is not.

@TheBrokenRail
Copy link
Contributor

Even on the server CowEntity does not fully represent a cow as it also has loot tables stored separately in the data-pack. And when talking technically I do say CowEntity instead of Cow. We are not making a gameplay tutorial, we are making a technical mappings set. Yes you would call a ChestBlockEntity a chest in normal game-play, but this isn't normal game-play, this is mod-making, where you don't want to confuse ChestBlock, ChestBlockEntity and the chest loot table. Yes, you should keep the suffix even if it is redundant for say creepers, because for other stuff the suffix is not redundant (LivingEntity, Entity, ItemEntity), and IMO it is better to be consistent in technical information than grammatically correct.

@Runemoro
Copy link
Contributor Author

Closing this since most are against it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion vote A vote on a name refactor
Projects
None yet
Development

No branches or pull requests