Jump to content

TileEntity revamp


target.san

Recommended Posts

Hello Forge team,

 

I have some thoughts on revamping tile entity model.

I'm offering to write a prototype.

And I'd like to hear if you're Ok with at least checking those changes or not.

My short list of ideas:

1. Make TileEntity class final and clean its interface a bit. You read it right. Close inheritance completely. The reason is, prohibiting inheritance from TileEntity, and most notably instanceof checks. I'll describe replacement and reasons below.

2. Create either interface or abstract class TileLogic, I haven't strictly decided yet. This thing will be the then returned by createTileEntity, instead of TileEntity instance.

3. TileEntity becomes container for TE's coordinates and some other basic things. Everything beyond this is delegated to TileLogic contained internally.

4. TileLogic will contain basic implementation of getBehavior method, as seen below:

public <T> T getBehavior(Class<T> clazz, ForgeDirection side) {
if (clazz.isInstance(this))
	return clazz.cast(this);
return null;
}

And this getBehavior will be used to cast TileLogic to IInventory, ISWidedInventory etc.

5. TileLogic reference will become a privete member of TileEntity, so no one can access it directly. Except reflection of course. TileEntity receives getBehavior delegation.

public <T> T getBehavior(Class<T> clazz, ForgeDirection side) {
return tileLogic.getBehavior(clazz, side);
}
public <T> T getBehavior(Class<T> clazz) {
return getBehavior(clazz, ForgeDirection.UNKNOWN);
}

6. With some possible trickery TE NBT storage might be made backwards-compatible, at least I don't see much problems.

 

Some reasoning on why I want to do this.

 

Zeroth, I clearly understand that this change is a deep intrusion into existing architecture. Though I think it's kinda right time to do such change, until Forge 1.8 is out to public.

 

First, I'm raising again this topic: http://www.minecraftforge.net/forum/index.php/topic,15339.0.html. So you can check previous iteration of discussion there.

 

Second. The current TileEntity design is rigid and inflexible with regards to current realities of modding. I see TEs with tons of methods, implementing tons of interfaces. Which are almost independent from one another. Second, I see many of those interfaces operating on TE sides. Some of them doing this in a strange manner. Let's just review IInventory and ISidedInventory. With sides operation moved into behavior logic, we're able to leave IInventory only and clean it, then just expose it on the sides we want.

 

Third. Aggregation vs inheritance. Yup, I'm raising this abstract criteria. Because I ate enough stuff in my programmer's career where inheritance created huuuge problems with extending systems. Though let's get to some more practical things. With TE behaviors implementation not restricted to inheritance, we can just create some container templates to serve us well. We create standard FluidHandler implementation, and a person can easily use it, without the need to inherit from it, just by adding member to his TileLogic. Ok, let's assume we need a tile which is both sided inventory and a tank. Today we need to implement all three interfaces, in one TE, by hand. Possibly with some help by inheriting from standard FluidHandler impl. With behaviors, we can aggregate standard FluidHandler, then aggregate standard InventoryHandler, then maybe create standard TankSlice/InventorySlice for one or both of them. Where those slice classes will present access to subset of inventory slots/fluid tanks, and then exposed to sides we need. Not writing and duping code, but composing already written pieces.

 

Fourth. Perfect forwarding. The case I'm doing now - a two-way spatial connector, which behaves like it's another entity adjacent to the opposite side of partner connector. I need to write tons of code. I need to reimplement every interface. I need to take into account internal logic. I need to do smart aggregation for IInventory. I think that other modders who implement any kind of indirect connection for other mods would agree. AE2's P2Ps as an example.

 

Fifth. No more @InterfaceList+@Interface+@Method annotations. Wanna make some behavior optional? Just return NULL when you don't want to return it. And of course aggregate, instead of inheriting it.

 

To sum up.

 

All this stuff might look too abstract. And the change to TileEntity looks really scary. As a downside, many vanilla interfaces will change. I hope they'll change for better. I'm not asking you guys to do this. I'm asking if you'll at least review this if I write the actual code, and move vanilla MC to this model. Because I'm not very eager to hear 'Nah, we won't be even reviewing that crap, not interested' in the end.

 

Again. I'm not asking for 100% approval. I'm asking if you're interested.

 

Thanks

 

Link to comment
Share on other sites

Forge likes to keep patches small.

 

Just my personal opinion, but I think you'll just be thrown out the front door. Lex has stated repeatedly he does not like to screw around with vanilla stuff unless absolutely necessary.

 

Is there a good reason for why you want these to be done?

Read the EAQ before posting! OR ELSE!

 

This isn't building better software, its trying to grab a place in the commit list of a highly visible github project.

 

www.forgeessentials.com

 

Don't PM me, I don't check this account unless I have to.

Link to comment
Share on other sites

As a downside, many vanilla interfaces will change.

You actually mean all of them ? All the blocks and all the inventories related to them. Probably also mob AI, redstone logic, world saves...

And who is going to maintain all those changes between Minecraft updates ?

 

You'd also break all mods related to TileEntity.

 

Aggregating is not a bad design idea. I apply it from time to time.

Just don't make it into the TileEntity. Make a ForgeTileEntity. Then you could probably suggest it.

Link to comment
Share on other sites

@luacs1998

 

I clearly understand this. Though IMO we're already at the point where we're stuck with Minecraft's architecture not suitable for modding. I just see how we're simply fighting with it. And we might change this. Or at least try this. Anyway it's better than to do nothing.

 

@GotoLink

 

I don't think that world saves would change. Or anything beyond TEs' structure.

Next, about what should change with such approach. All 'tile instanceof X' should be replaced with getBehavior. It would drop some interfaces like ISidedInventory, making them simply obsolete. Existing tile entities will be modified to accomodate.

 

ForgeTileEntity. We would need to enforce other modders to use it, instead of TileEntity. Enforce them to use method invocation instead of typechecks and casts. I guess you remember what happened with attempts to introduce dynamic block IDs not from core, but from side. Until Mojang moved to dynamic IDs in 1.6.

 

Frankly speaking, I don't expect that my changes will be accepted instantly. But this can be a good starting point to communicate with Mojang guys so they make these changes on their side. Not some 'idea', but an almost-ready prototype. We have Searge and Dinnerbone there AFAIK. We can try to speak with them. I remember that 1.6 update when we finally got block names instead of IDs. And I suspect this was not without LexManos or someone else.

Link to comment
Share on other sites

I have a purely personal opinion that the Forge team could've already rewritten MC into a real eyecandy, if they had full ownership over sources. 'Cause modders community produces 1-2 orders of magnitude more content than Mojang guys. Unfortunately, not in this world, unless MS really closes all doors and Forge would just have no options than going rogue. Then it would just have no boundaries for evolving.

Link to comment
Share on other sites

@diesieben07

 

I clearly understand the problem with rogue patches to code achieved via deobfuscation. And as I said, it's a 'would be' scenario. Unless Forge team decides to make their own fork, he-he.

 

Scala or Kotlin won't give you dynamic behavior change. And won't give you entity-wide siding support. And no proper composition. So ugly interfaces with 'side' parameter in each method will remain. As for Minecraft being as it is, IMO there's possibility to influence MC core team. Though you have more info I think.

Link to comment
Share on other sites

A sample for TE which changes its set of behaviors is any multipart. Any multipart-enabled pipe or power conduit. See my prev. topic on this issue.

Basically, it's anything complex enough to be composed of several parts and thus having several behaviors which might appear and vanish.

 

As for neccessity. Sure, we can live without it... but it's a painful life. And the solution I propose won't only allow to do some new stuff, but also simplify existing stuff significantly.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.



  • Recently Browsing

    • No registered users viewing this page.
  • Posts

    • I made a basic homing arrow, which I am going to refine further, but as of now I have the problem of the Arrow not rendering properly. It works fine on the server side, but on the client it always appears as if the arrow was flying normally, until it synchs and teleports around.   [https://gemoo.com/tools/upload-video/share/661346070437036032?codeId=MpmzxaW0pBpE1&card=661346066800603136&origin=videolinkgenerator] the white particles are created by the entity every tick on the server side and represent its actual flying path.   My best guess is that this behaviour is caused, because minecraft appears to try to predict the projectiles path on the client side instead of constantly synching (perhaps something to do with it implementing the TracableEntity interface??). I am thinking I need help with either 1. Getting the client to use my custom Tick function in its path prediction, or 2. (the less elegant solution) getting the game to synch up the direction, position and movement etc. every tick.     Note that my custom arrow class extends AbstractArrow. everything important it does: private static final EntityDataAccessor<Integer> TARGET_ENTITY = SynchedEntityData.defineId(ReachArrow.class, EntityDataSerializers.INT); @Override public void addAdditionalSaveData(CompoundTag pCompound) { super.addAdditionalSaveData(pCompound); pCompound.putInt("TargetEntity", this.getTargetEntity()); } @Override public void readAdditionalSaveData(CompoundTag pCompound) { super.readAdditionalSaveData(pCompound); this.setTargetEntity(pCompound.getInt("TargetEntity")); } @Override protected void defineSynchedData() { super.defineSynchedData(); this.entityData.define(TARGET_ENTITY, -1); } @Override public void shootFromRotation(Entity pShooter, float pX, float pY, float pZ, float pVelocity, float pInaccuracy) { LivingEntity target = ReachArrow.findNearestTarget(this.level(),(LivingEntity) pShooter,50d); if(pShooter instanceof LivingEntity && target !=null){ //pShooter.sendSystemMessage(Component.literal("setting id "+target.getId()+" as target")); setTargetEntity(target.getId()); //pShooter.sendSystemMessage(Component.literal("target set")); } super.shootFromRotation(pShooter, pX, pY, pZ, pVelocity, pInaccuracy); } public static LivingEntity findNearestTarget(Level world, LivingEntity shooter, double range) { AABB searchBox = shooter.getBoundingBox().inflate(range); List<LivingEntity> potentialTargets = world.getEntitiesOfClass(LivingEntity.class, searchBox, EntitySelector.NO_SPECTATORS); LivingEntity nearestTarget = null; double closestDistance = Double.MAX_VALUE; for (LivingEntity potentialTarget : potentialTargets) { if (potentialTarget != shooter && potentialTarget.isAlive()) { double distance = shooter.distanceToSqr(potentialTarget); if (distance < closestDistance) { closestDistance = distance; nearestTarget = potentialTarget; } } } return nearestTarget; } I tried fixing the problem by storing the Target using SynchedEntityData, which not only didn't fix the problem, but also added unwanted, blue particles to the arrow (like on tipped arrow) Thank you in advance for any help or hints, I am quite new at this so you could probably help me a lot. :)
    • When trying to load Craft to Exile 2, game crashes and this error message pops up:   https://api.mclo.gs/1/raw/B2oYte0
    • FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':processResources'. > Could not copy file 'C:\Users\jedil\Downloads\forge-1.20-46.0.14-mdk\src\main\resources\META-INF\mods.toml' to 'C:\Users\jedil\Downloads\forge-1.20-46.0.14-mdk\build\resources\main\META-INF\mods.toml'.    > Failed to parse template script (your template may contain an error or be trying to use expressions not currently supported): startup failed:      SimpleTemplateScript1.groovy: 1: Unexpected input: '(' @ line 1, column 10.         out.print("""# This is an example mods.toml file. It contains the data relating to the loading mods.                  ^   This is my mods.toml script: # This is an example mods.toml file. It contains the data relating to the loading mods. # There are several mandatory fields (#mandatory), and many more that are optional (#optional). # The overall format is standard TOML format, v0.5.0. # Note that there are a couple of TOML lists in this file. # Find more information on toml format here: https://github.com/toml-lang/toml # The name of the mod loader type to load - for regular FML @Mod mods it should be javafml modLoader="javafml" #mandatory # A version range to match for said mod loader - for regular FML @Mod it will be the forge version loaderVersion="${46.0.14}" #mandatory This is typically bumped every Minecraft version by Forge. See our download page for lists of versions. # The license for you mod. This is mandatory metadata and allows for easier comprehension of your redistributive properties. # Review your options at https://choosealicense.com/. All rights reserved is the default copyright stance, and is thus the default here. license="${All Rights Reserved}" # A URL to refer people to when problems occur with this mod #issueTrackerURL="https://change.me.to.your.issue.tracker.example.invalid/" #optional # A list of mods - how many allowed here is determined by the individual mod loader [[mods]] #mandatory # The modid of the mod modId="${MCRefined}" #mandatory # The version number of the mod version="${1.0.0}" #mandatory # A display name for the mod displayName="${Minecraft Refined}" #mandatory # A URL to query for updates for this mod. See the JSON update specification https://docs.minecraftforge.net/en/latest/misc/updatechecker/ #updateJSONURL="https://change.me.example.invalid/updates.json" #optional # A URL for the "homepage" for this mod, displayed in the mod UI #displayURL="https://change.me.to.your.mods.homepage.example.invalid/" #optional # A file name (in the root of the mod JAR) containing a logo for display #logoFile="examplemod.png" #optional # A text field displayed in the mod UI #credits="" #optional # A text field displayed in the mod UI authors="${me}" #optional # Display Test controls the display for your mod in the server connection screen # MATCH_VERSION means that your mod will cause a red X if the versions on client and server differ. This is the default behaviour and should be what you choose if you have server and client elements to your mod. # IGNORE_SERVER_VERSION means that your mod will not cause a red X if it's present on the server but not on the client. This is what you should use if you're a server only mod. # IGNORE_ALL_VERSION means that your mod will not cause a red X if it's present on the client or the server. This is a special case and should only be used if your mod has no server component. # NONE means that no display test is set on your mod. You need to do this yourself, see IExtensionPoint.DisplayTest for more information. You can define any scheme you wish with this value. # IMPORTANT NOTE: this is NOT an instruction as to which environments (CLIENT or DEDICATED SERVER) your mod loads on. Your mod should load (and maybe do nothing!) whereever it finds itself. #displayTest="MATCH_VERSION" # MATCH_VERSION is the default if nothing is specified (#optional) # The description text for the mod (multi line!) (#mandatory) description='''${Minecraft, but it's, like, better.}''' # A dependency - use the . to indicate dependency for a specific modid. Dependencies are optional. I tried using --scan or --stacktrace, those were no help. I also tried Ctrl+Alt+S, the template I used did not appear. HELP
    • They were intended to be used on tutorial posts so that people could easily find tutorials based on their skill level, but instead the tags were abused for unrelated things that made the original intent useless... for example, people often posted crash reports with the "beginner" tag, so instead of finding tutorials for beginners, you got crash reports showing up in searches.
    • The crash says: Exception caught when registering wandering trader java.lang.NullPointerException: Cannot invoke "net.minecraft.world.entity.Entity.m_9236_()" because "entity" is null at com.telepathicgrunt.repurposedstructures.misc.maptrades.StructureSpecificMaps$TreasureMapForEmeralds.m_213663_(StructureSpecificMaps.java:53) ~[repurposed_structures-7.1.15+1.20.1-forge.jar%23708!/:?] at jeresources.collection.TradeList.addMerchantRecipe(TradeList.java:58) ~[JustEnoughResources-1.20.1-1.4.0.247.jar%23630!/:1.4.0.247] JustEnoughResources is mentioned, too Does it work without one of these mods?
  • Topics

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.