Spongeapi: TileEntity -> BlockEntity

[kashike]

What we call TileEntity should be called BlockEntity for consistency:

• we have BlockType, not TileType - Tile was renamed to Block in Minecraft years ago
• vanilla and the community refer to these as "BlockEntity"/"block entity", except in some legacy cases (doTileDrops gamerule, for example)

---

Please react with :+1: if you are in favour, and :-1: if you are not in favour.

Deadline for feedback and voting is 12:00pm PDT on Monday (06/05/2019).

[phit]

I honestly don't care much either way, however I voted against this for the simple reason that the implementation uses MCP and as long it won't be changed there, we would be mixing these two which just adds needless confusion in my opinion.

[kashike]

The API doesn't rely on MCP and thus names shouldn't rely on MCP.

[phit]

yeah, but the API is still implemented using MCP mappings

[liach]

We get names significantly better than those in other mappings and even than official vanilla names, like LiteralText over TextComponent. I believe using more descriptive names like BlockEntity (which is fairly widely accepted as of now) is beneficial to the Sponge API.

[pie-flavor]

The reasoning behind much of Sponge's naming is to use names that make sense, not names that other APIs use (e.g. event verb-noun naming). So yeah, MCP shouldn't mean anything here IMO.

[BrainStone]

Where is all that MCP trash talking coming from?
MCP has been pretty much essential to the MC modding community for the longest so I really don’t understand the hard feelings expressed here.

And saying to keep naming consistent with MCP and therefore Forge is a good thing.

Now another thing that’s really bothering me about these votes in recent days is that it seems that most pressing issues seem to be to make pointless renames that do nothing. The code doesn’t care what the class is called.
Call me old fashioned but I strongly believe that renaming stuff for renaming sake after said name has been established for years is not beneficial, unless the name itself is either plain wrong or severely misleading. Like a boolean value which name is reversed to it’s logic. (Great example of a field that needs renaming is MCP’s World#isRemote. As remote is commonly used for the server side. Though in best case it’s ambiguous)

Now to reiterate, while MCP is not a direct necessity for Sponge, it’s more or less a type of common standard. And I believe it should be followed to keep naming in the community consistent, which I believe to be far more important than renaming an established name to a slightly more descriptive name.

Now this renaming should be taking place first in MCP. Though even there I would oppose it, as it’s again an established name. And renaming it would be a slight disambiguation at best. And breaking legacy over such a tiny thing doesn’t make sense to me.

[liach]

Note that Sponge is not Bukkit. There is no reason to keep legacy names; Notch initially called all blocks "tile"; would you still call BlockType TileType instead?

And saying to keep naming consistent with MCP and therefore Forge is a good thing.

If you want to stick with MCP or forge, look at ModCoderPack/MCPBot-Issues#815 ModCoderPack/MCPBot-Issues#816 and ModCoderPack/MCPBot-Issues#817. All these proposals have been done in sponge api in its birth days, yet it turns out sponge's naming scheme is efficient and comprehensible.

Now to reiterate, while MCP is not a direct necessity for Sponge, it’s more or less a type of common standard.

Moreover, MCP is not the only standard. Look up on your own; there is also bukkit mappings and fabricmc yarn mappings.

And breaking legacy over such a tiny thing doesn’t make sense to me.

1.14 is going to break. Commands, data packs, data fixers, async resource reloading, async lighting, mob brain... You should not try to preserve "legacy", especially one with adverse effects, in this case.

[johnfriedrich]

I would assume 90% (if not more)of all devs are using SpongeForge not SpongeVanilla so they are working with Forge somehow. Thats the reality. Even though the new name would be better, the change should be made at mcp first. Only changing it here would cause a lot of unnecessary confusion. Just my two cents

[gabizou]

One very important factor that I need to mention for this issue (which is likely to be re-added to the issue description):

If the change is made from TileEntity -> BlockEntity, so will the configs need to be changed for Sponge's configurations (tracker configs, error configs, etc.) referencing tile-entity etc. being changed over to block-entity. While this may not seem like much, it does mean performing that change will also require us to actually make a proper version upgrader for our configs to migrate all existing configs between API versions (and this will also play into our additions/changes as features are implemented, corrects implemented, optimizations added, etc.). So while this is an overall API change, this does have the side effect of changing our public facing configuration options being exposed through SpongeCommon.

[phit]

While this may not seem like much, ...

it will just confuse literally every user, because all of modded minecraft calls them tile entities and always has, great..

[johnfriedrich]

it will just confuse literally every user, because all of modded minecraft calls them tile entities and always has, great..

This

[Katrix]

I also agree with the fact that if this change should be made, it needs to first be made in MCP. IF we do make this change we can't just be quiet about it to the end users. It needs to me made very clear to everyone that uses Sponge that what other platforms calls tile entities, Sponge calls block entities.

While it may be true that the general community calls the block entities (I really doubt this), the majority of the developer community calls them tile entities. I won't be surprised if such a change, would lead to a common question being asked again and again by new developers.

Overall, if we do change this, I don't think it will be a mostly one time cost like it is for Forge to switch from prefix to suffix. IMO such a change should come from MCP, with Sponge then after some time following that direction.

[liach]

it will just confuse literally every user, because all of modded minecraft calls them tile entities and always has, great..

Lol no! See this: https://github.com/StellarHorizons/Galacticraft-Rewoven/blob/e32d407a77f851dc50318b0d42bcab2afd173354/src/main/java/com/hrznstudio/galacticraft/blocks/machines/basicsolarpanel/BasicSolarPanelPartBlockEntity.java#L11

Modders do call these things "block entity".

I also agree with the fact that if this change should be made, it needs to first be made in MCP.

IMO such a change should come from MCP, with Sponge then after some time following that direction.

We should be unconcerned with MCP. We have done ModCoderPack/MCPBot-Issues#815 ModCoderPack/MCPBot-Issues#816 and ModCoderPack/MCPBot-Issues#817 way before the proposals are even forwarded there. Moreover, other Minecraft mappings exist, such as bukkit nms (which is antiquated and should not be a good model to follow) and FabricMC yarn (Used by that example mod code I linked above)

IF we do make this change we can't just be quiet about it to the end users.

We have names that match that on Minecraft Wiki and official terms. We get sponge docs which explain this as well, so things are fine.

[phit]

Modders do call these things "block entity".

and some call them potato, that doesn't really matter, there will be always exceptions to cherry pick
310,000 code results https://github.com/search?l=Java&q=tileentity&type=Code
5,100 code results https://github.com/search?l=Java&q=blockentity&type=Code

[Katrix]

I don't feel like those recent name changes in MCP can be compared against too much. They are more a distraction that anything else. Those are stylistic changes. How it's written might have changed, but everything still has it's same name. And sure, Sponge can do different stuff from MCP there. It's not like Forge and Sponge uses the same coding style.

Sure, if the issue here wasn't talking about just TileEntity -> BlockEntity, but MCP name -> Mojang name in every place where we knew what they called their stuff, changing the name if we did discover it, then sure, I'd be in favor. But not just for a single name like this which I see as something bound to create confusion.

Sure, we can add a note to the docs. I see a few hurdles to get over there though. 1. How many people will know where to look for that note. We can't put it under the block entity section, because that's not what people think they're looking for. 2. How many people will read the docs before asking in a more direct support channel like the Discord.

[liach]

Let's just take 1.13 commands for example.

The tendency to stay on old things is not good. 1.13 even broke a lot of vanilla users with its new brigadier and new commands, but it does not mean that we should add old, less flexible commands back to the game.

and some call them potato, that doesn't really matter, there will be always exceptions to cherry pick
310,000 code results https://github.com/search?l=Java&q=tileentity&type=Code
5,100 code results https://github.com/search?l=Java&q=blockentity&type=Code

Moreover, github search is inherently limited. See the search result for execute, one of the most used minecraft commands, on github:
https://github.com/search?q=execute+language%3Amcfunction&type=Code

And compare that with the search result for bossbar, which is used significantly less often as far as I know:
https://github.com/search?q=bossbar+language%3Amcfunction&type=Code

[phit]

I don't even know what mcfunction is or how it relates to what mod users are used to

[liach]

that demonstrates your github search result, or the way you gather evidence, is questionable. anyways, let's just vote.

[johnfriedrich]

It's not about breaking things. That doesn't matter. In the end it's replace all and fixed in three seconds. At least when updating a mod. (Yeah like gabi said, configs and so on are a huge deal). But it's not even that. It's about consistency. If Forge modders use TileEntity all day, why should they randomly start using BlockEntity just for Sponge plugin developing? As mentioned above, Forge and Sponge mod/plugin developing are tied together. The same (server)dev uses both in most cases. That's why consistency is important. It's a random and unnecessary change. Let MCP change it and Sponge can follow. Especially newbies could get confused even more when starting working with Forge/Sponge. But yeah, let the vote decide

[pie-flavor]

Because Sponge isn't Forge. Sponge might be implemented on top of Forge but that doesn't mean anything. You don't start using C# naming conventions in your Python code just because you know it'll be running on IronPython. If you want an API as close to Forge as possible, just use Forge. Also, 'new developers' are overwhelmingly new to development itself, not Forge developers trying out Sponge. So there would not be confusion. Finally, it's not just a community thing - the one unobfuscated aspect of Minecraft, NBT names, clearly calls them block entities and not tile entities. Block entities are therefore the correct name, and the only reason Forge modders still use 'tile entities' is because the mappings haven't been changed yet.

Sponge tries to be correct, not to do what other people do. Forge naming means absolutely nothing IMO.

[axle2005]

Its is possible to have a Depreciation options for this? Do the initial update so that TileEntity and BlockEntity both exist with a Depreciation warning on TileEntity? This would allow plugin developers to identify where exactly in our code things will break without holding the community back due to lack of plugins again...

I know several of mine use TileEntity, but ClearMob would be the only one that has any kind of use base... but plugins like GriefPrevention I am sure could benefit from a warning in an IDE rather than a flat out "It's dead Jim"

I realize this would be a lot more work than simply deprecating a method... I am just thinking of how a lot of plugins aren't being updated a lot lately.. this would at least allow the community time to update or replace any plugins that are no longer supported going into API 8.

[Zidane]

@axle2005 All deprecation has been removed in API 8. If this change happens, it would be a clean break. Other changes in API 8 will have already broke your plugins (potentially but likely) so that isn't a strong point here.

[axle2005]

Thats fair... I fully expect larger plugins to right out die... much like the transition from API 5 to 7 where Cause was removed from a lot of methods.

[Zidane]

@axle2005 unfortunately it is the nature of evolution. The base game's changes meant while a lot of the API was still sound (due to prior planning), a lot wasn't. Since things were going to break, we decided to get a lot of breaking changes out of the way.

AKA we're going to rip the bandaid off.

[macks2008]

seeing as and a lot of plugins are going to break from the removal of depreciation anyway, my concern with "breaking unmaintained or under maintained code" is probably irrelevant here, because it's happening regardless. That being my biggest objection, I see no reason to postpone this till the next major code restructuring or anything like that. The code may not care what we call blocks (that's what they are: blocks), but if I'm developing a plug-in (I'm not right now... But I might be in the near future), I'd rather call a 1 m³ voxeloid/cube/whatever a "block" than a "tile". The Band-Aid (as Zidane called it) is going to come off sooner or later, and it sounds like it's already half way off.

[BrainStone]

Though the one thing I really don’t understand is where’s the benefit in creating a discrepancy between Sponge and MCP (and yes they are inherently linked. SpongeForge is a thing and people develop stuff that uses both Sponge and Forge at the same time) for fixing a slightly odd name, that is well known and pretty much everyone in the scene knows.

Why make it harder for devs over such small and petty things?

[dualspiral]

and yes they are inherently linked

No, that's just plain wrong. There is no link between SpongeAPI and MCP. Lantern uses SpongeAPI and has no link with MCP. It's the implementations that do, and this is nothing to do with the implementations. If we were to make SpongeFabric (we're not, but for the sake of argument let's hypothesise) and that SpongeForge was discontinued (again, that's not going to happen, hypothetical situation yet again), do we have to morph our API names to reflect their mappings? You can bet that the answer is _not at all_.

Just because MCP, or Fabric, or some other mapping, names something one way, doesn't mean we have to follow suit - and we shoudn't feel bound to that. It's our tool of choice for implementation, NOT naming. Similarly, there are plenty of developers that don't use Forge or MCP and rely purely on Sponge for their modding needs, why should they be bound to modding names. (I know the counter argument is that they don't know anything different and so we should cater for those that do - but that's a terrible argument as it just exposes more people to the legacy way of doing things rather than trying to actively change things for the better.)

We shouldn't feel shackled by what are bad naming decisions, either in MCP or elsewhere. We should be naming things in a way that makes sense rather than for legacy reasons. We are our own project. However, we are being receptive to the fact that we're asking people what they think - hence this vote.

For the record, I don't care which way this goes. My gut feeling originally was to keep the original name, but I've recinded my vote and will remain neutral on this.

[ryantheleach]

I think block entities are very different to regular entities, and could be easily confused for FallingBlock entities which are an entirely different construct.

That said, BlockEntities seems more natural.

[VcSaJen]

I vote to postpone this issue until 1.15 is released.

[Zidane]

@VcSaJen For what reason?

[kashike]

Thank you for the input, everyone. This issue is now closed for voting.

[gabizou]

I'd like to summarize the overall change as a result of the voting:

• org.spongepowered.api.block.tileentity.TileEntity -> org.spongepowered.api.block.entity.BlockEntity (and all the subclasses get package moved too)
• org.spongepowered.api.block.tileentity.TileEntityArchetype -> org.spongepowered.api.block.entity.BlockEntityArchetype
• org.spongepowered.api.block.tileentity.TileEntityType -> org.spongepowered.api.block.entity.BlockEntityType
• org.spongepowered.api.volume.TileEntityVolume -> org.spongepowered.api.volume.BlockEntityVolume (and all the prefixes involved.

All the named methods involving TileEntity will be renamed to BlockEntity as well.

All configuration options in implementation will obey the BlockEntity naming style (which will be coming later, as we'll have to get implementation working before we can work on updating/upgrading configurations.