Jump to content

Recommended Posts

Posted

Hello everyone.

 

As I understand, the common way to add some custom logic to TileEntity and expose it as API is to add static interface. It's a common way to add behavior at compile-time.

The problem is, there's no common way to add such behavior in runtime. If some mod gives such opportunity, it adds its own interface with method like getEUSink or like.

What I propose is to add such method to TileEntity at the Forge level. So we'll have something like:

object getBehavior(Class clazz)

Which will return required projection or adaptor for tile entity, if it exists. If TileEntity implements required "clazz" directly, then this is returned.

Pros:

1. Unified way for tile entities to communicate

2. Modular tile entities can change their behavior in runtime easily

Cons:

1. Theoretical performance degradation due to more type casts

Posted

Your idea is not better than

object instanceof clazz
//or
clazz.isAssignableFrom(cls)

In your example you can't make same object become instanceof clazz on current tick, and then unbecome on next tick.

 

And you can already change "Modular tile entities" behavior at runtime.

Please be more specific. If you're telling about some existing implementations, then I'm not arguing that. I'm just suggesting a more centralized mechanic for this. The problem is, if mod A provides modular TE's, and mod B provides modular TE's, then these are two different kinds of modular TE's.

Posted

In your example you can't make same object become instanceof clazz on current tick, and then unbecome on next tick.

You mean something like this ?

AbstractBehavior behavior;

public void tick(){
     if(behavior instanceof DefaultBehavior){
         behavior = new RandomBehavior(this);
    }else{
         behavior = new DefaultBehavior(this);
    }
    behavior.behave();
}

 

Please be more specific. If you're telling about some existing implementations, then I'm not arguing that. I'm just suggesting a more centralized mechanic for this. The problem is, if mod A provides modular TE's, and mod B provides modular TE's, then these are two different kinds of modular TE's.

Take my example, i don't get how your method would help you or anyone else to "communicate" with it.

Posted

Okay, here are some notes on why that getBehavior would be useful.

First, you won't see benefits from this on basic TE's - only on compound ones.

Second, I don't think I would be able to give some short and clean code samples due to 'first'.

Here's rough, stripped version of TileGenericPipe class from BuildCraft. It implements 10 interfaces.

 

 

public class TileGenericPipe extends TileEntity implements IPowerReceptor, IFluidHandler, IPipeTile, IOverrideDefaultTriggers, ITileBufferHolder,
	IDropControlInventory, IPipeRenderState, ISyncedTile, ISolidSideTile, IGuiReturnHandler {

public TileGenericPipe() {...}

@Override
public void writeToNBT(NBTTagCompound nbt) {...}

@Override
public void readFromNBT(NBTTagCompound nbt) {...}

@Override
public void invalidate() {...}

@Override
public void validate() {...}

@Override
public void updateEntity() {...}

public void initialize(Pipe pipe) {...}

@Override
public IPipe getPipe() {...}

@Override
public PowerHandler.PowerReceiver getPowerReceiver(ForgeDirection side) {...}

@Override
public void doWork(PowerHandler workProvider) {...}

public void scheduleNeighborChange() {...}

@Override
public int injectItem(ItemStack payload, boolean doAdd, ForgeDirection from) {...}

@Override
public PipeType getPipeType() {...}

@Override
public Packet getDescriptionPacket() {...}

public void sendUpdateToClient() {...}

@Override
public LinkedList<ITrigger> getTriggers() {...}

@Override
public void blockRemoved(ForgeDirection from) {...}

public TileBuffer[] getTileCache() {...}

@Override
public void blockCreated(ForgeDirection from, int blockID, TileEntity tile) {...}

@Override
public int getBlockId(ForgeDirection to) {...}

@Override
public TileEntity getTile(ForgeDirection to) {...}

@Override
public boolean isPipeConnected(ForgeDirection with) {...}

@Override
public boolean doDrop() {...}

@Override
public void onChunkUnload() {...}

@Override
public int fill(ForgeDirection from, FluidStack resource, boolean doFill) {...}

@Override
public FluidStack drain(ForgeDirection from, int maxDrain, boolean doDrain) {...}

@Override
public FluidStack drain(ForgeDirection from, FluidStack resource, boolean doDrain) {...}

@Override
public boolean canFill(ForgeDirection from, Fluid fluid) {...}

@Override
public boolean canDrain(ForgeDirection from, Fluid fluid) {...}

@Override
public FluidTankInfo[] getTankInfo(ForgeDirection from) {...}

public void scheduleRenderUpdate() {...}

public boolean addFacade(ForgeDirection direction, int blockid, int meta) {...}

public boolean hasFacade(ForgeDirection direction) {...}

public boolean dropFacade(ForgeDirection direction) {...}

@Override
public PipeRenderState getRenderState() {...}

@Override
@SideOnly(Side.CLIENT)
public IIconProvider getPipeIcons() {...}

@Override
public IClientState getStateInstance(byte stateId) {...}

@Override
public void afterStateUpdated(byte stateId) {...}

@Override
@SideOnly(Side.CLIENT)
public double getMaxRenderDistanceSquared() {...}

@Override
public boolean shouldRefresh(int oldID, int newID, int oldMeta, int newMeta, World world, int x, int y, int z) {...}

@Override
public boolean isSolidOnSide(ForgeDirection side) {...}

public boolean hasPlug(ForgeDirection side) {...}

public boolean removeAndDropPlug(ForgeDirection side) {...}

public boolean addPlug(ForgeDirection forgeDirection) {...}

public int getBlockId() {...}

@Override
public World getWorld() {...}

public boolean isUseableByPlayer(EntityPlayer player) {...}

@Override
public void writeGuiData(DataOutputStream data) throws IOException {...}

@Override
public void readGuiData(DataInputStream data, EntityPlayer sender) throws IOException {...}
}

 

 

Issues I can personally see here:

1. All pipe responsibilities in one class

2. All pipes need to be potential item, liquid and power pipes. Or, this part should be split into three separate TE's.

3. All pipe behaviors are statically described in class contract, i.e. you can't add new responsibilities without modifying class.

If getBehavior or something like is present, all interfaces which aren't directly related to how pipe works can be extracted to separate classes.

This will:

1. Lighten up core class

2. Allow each ppe to implement only interfaces it really needs, i.e. power pipe won't need to be item and fluid pipe at the same time

Moreover, let's check LogisticsPipes pipe. Let's imagine we need chassis module which provides BC power to connected blocks. With current state of things, you'll need

a) Add IPowerEmitter interface to TE class

b) implement it directly in TE class

c) Process both cases when we have module and when not, even for pipes which aren't chassis.

 

To sum up,

+ Allows to split TE implementation into several independent parts, and add such parts more easily

+ Allows to modify TE behaviors at runtime, depending on TE state, without need of stub imterface implementations

+ Allows one block to mimick another, like Transfactor Interface from Thaumic Tinkerer, but without limiting number of mimicked behaviors

- Requires one more method

- Might require to use class->handler map internally

* Migration might be easened with following code in default implementation:

if (this instanceof clazz)
  return this;

* The idea might be extended

  object getBehavior(Class clazz, ForgeDirection side)

  This will allow to have different behaviors on sides, and side-independent ones for UNKNOWN direction

 

  • 1 month later...
Posted

I don't really see the benefit of this rewrite. Besides the fact that refactoring the modder's code is a modder responsibility. Theirs, not ours.

Also, if you just want a pipe that handles liquids, just implement the more limited set of interfaces in your class. Then use your class wherever a liquid pipe is needed. That's the beauty of interfaces. Your class can adhere to only the contracts it needs to perform its job.

A backhoe might have interfaces for wheeledVehicle, contructionEquipment, diggerMachine, HydrolicsUser, and others. While a car might implement wheeledVehicle, roadWorthy, comfortProvider, etc. why make a car into a backhoe when that is not its job?

Posted

As I wrote earlier, the main benefit is the ability to modify set of behaviors TE supports in runtime.

For now I'm mostly looking at LogisticsPipes as an example. There, chassis pipe should support _all_ possible interfaces beforehand.

Actually, any entity which can or cannot be something depending on state would benefit from such change.

Posted

Okay, here are some notes on why that getBehavior would be useful.

First, you won't see benefits from this on basic TE's - only on compound ones.

Second, I don't think I would be able to give some short and clean code samples due to 'first'.

Here's rough, stripped version of TileGenericPipe class from BuildCraft. It implements 10 interfaces.

 

Issues I can personally see here:

1. All pipe responsibilities in one class

2. All pipes need to be potential item, liquid and power pipes. Or, this part should be split into three separate TE's.

3. All pipe behaviors are statically described in class contract, i.e. you can't add new responsibilities without modifying class.

If getBehavior or something like is present, all interfaces which aren't directly related to how pipe works can be extracted to separate classes.

All this is because they didn't abstract enough.

A generic pipe for me is only a means of transportation.

Object to be transported should simply be wrapped with their transportation method, which should be called as the pipe comes into contact. Example:

public class GenericPipe extends TileEntity{
Vec3 vec3;
List<Transportable> transports = new ArrayList();
List<ContactHandler> lookers = new ArrayList();
public GenericPipe(){
    vec3 = Vec3.createVectorHelper(this.posX,this.posY,this.posZ);
}

public void addTransportFor(Transportable object, ContactHandler handler){
     transports.add(object);
     lookers.add(handler);
}

public boolean isInContactWith(Transportable object){
    for(ContactHandler handler:lookers){
        if(handler.canTouch(object, this)
            return true;
    }
}

public void send(Direction dir, Speed speed){
    for(Transportable object:transports){
        if(isInContactWith(object)){
            object.transportTo(dir, speed, this.vec3, this.worldObj);
        }
    }
}

}

 

  object getBehavior(Class clazz, ForgeDirection side)

  This will allow to have different behaviors on sides, and side-independent ones for UNKNOWN direction

So, let say we are in this case:

TileEntity tileEntity = world.getTileEntity(x,y,z);//some random code to get a TileEntity instance
Object obj = tileEntity.getBehavior(SomeBehavior.class, Side.WTV);//you call that method you suggested
obj. //? now what ?

And compare it with the current situation:

TileEntity tileEntity = world.getTileEntity(x,y,z);//some random code to get a TileEntity instance
if(tileEntity instanceof SomeBehavior){
  ((SomeBehavior)tileEntity).doSomething(Side.WTV);
}

I don't see any improvement from your suggestion...

Posted

  object getBehavior(Class clazz, ForgeDirection side)

  This will allow to have different behaviors on sides, and side-independent ones for UNKNOWN direction

So, let say we are in this case:

TileEntity tileEntity = world.getTileEntity(x,y,z);//some random code to get a TileEntity instance
Object obj = tileEntity.getBehavior(SomeBehavior.class, Side.WTV);//you call that method you suggested
obj. //? now what ?

And compare it with the current situation:

TileEntity tileEntity = world.getTileEntity(x,y,z);//some random code to get a TileEntity instance
if(tileEntity instanceof SomeBehavior){
  ((SomeBehavior)tileEntity).doSomething(Side.WTV);
}

I don't see any improvement from your suggestion...

 

I'm not good at generics. Sketch solution for TileEntity with more proper signature would look like this:

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

Next, we're having some class with dynamic set of behaviors

Map<Class, Object> behaviors = new HashMap<Class, Object>();

public <T> T getBehavior(Class<T> clazz, ForgeDirection side) {
Object obj = behaviors.get(clazz);
if (obj != null)
	return clazz.cast(obj);
return super.getBehavior(clazz, side);
}

And that's how we can use this:

ISomeBehavior b = tileEntity.getBehavior(ISomeBehavior.class);
if (b != null)
... do some nifty stuff ...

The main advantage is that we'll be using single approach for all cases. It won't matter whether that behavior is a superclass of TE or some internal aggregate - we'll need to call getBehavior.

Next, let's remember about such a block as Transfactor Interface from Thaumic Tinkerer. With current approach, author needed to implement every possible interface by hand. With getBehavior, he can just delegate to linked entity's getBehavior - less than 10 lines of code to rule them all.

Posted

Several things to note:

[*]Your example getBehavior method never uses the side argument. It is therefore useless as an example of anything except a spiffy class cast.

[*]Your map would not work in most situations as many instances of a class exist, duplicate keys will abound.

[*]Your second code snippet might as well be static as it makes no reference to this at all. What's it's purpose again?

[*]Your third snippet boils down to, try {ISomeBehavior b = (ISomeBehavior)tileEntity;} catch(Exception e) {b = null; }

[*]What's the advantage to your method? I still see none.

 

Am I missing the obvious?

Posted

Your example getBehavior method never uses the side argument. It is therefore useless as an example of anything except a spiffy class cast.

Yes, because it's an example implementation. Derivatives might expose different behaviors on different sides if they want.

Your map would not work in most situations as many instances of a class exist, duplicate keys will abound.

There's no static modifier, so it's an instance variable. If I didn't add access modifier, it wouldn't make map static.

Your second code snippet might as well be static as it makes no reference to this at all. What's it's purpose again?

See prev item, it references instance map.

Your third snippet boils down to, try {ISomeBehavior b = (ISomeBehavior)tileEntity;} catch(Exception e) {b = null; }

It doesn't. Because, depending on real TE type, it might return behavior which isn't TE's implemented interface or superclass.

And my main point is that we'll have single way to access both inheritance-based behaviors and aggregation-based ones.

What's the advantage to your method? I still see none.

 

Am I missing the obvious?

Yup, you are.

 

As another example, proposed approach would allow to implement multipart blocks with inherent support for parts with complex logic. At example, ordinary multiparts form one tile entity type. But if you need something like multipart wire, you have to make your own tile entity - because you need several behaviors exposed as interfaces, statically. With proposed approach, there would be no need for separate TE class - only some code which allows block parts to expose their behaviors on block sides. Modularity in action. BTW, this is one of many places for side argument which you thought as excessive.

Posted

In regards to my second point: Your hashmap requires unique keys. Classes and interfaces tend not to be unique, where instances usually are. Therefore, you could never create more than one instance of any class or interface in your map and successfully register the new one. Your best hope would be for a HashMap(Class<?>,Set<Object>) to represent such a mapping.

 

Another thing, your idea represents breaking an object (Like a TE) into sides. But a TE normally requires and assumes all six sides belong to it. You'd get farther along by creating MicroTEs where each gets an isosceles pyramid shaped area of the block. Then you could make aggregates of these to fill a block.

TileEntiry myWierdBlock = new AggregateTE(MicroTE ob_up, MicroTE ob_down,
    MicroTE ob_north, MicroTE ob_south,
    MicroTE ob_east, MicroTE ob_west);

 

That would at least be a start.

Posted

In regards to my second point: Your hashmap requires unique keys. Classes and interfaces tend not to be unique, where instances usually are. Therefore, you could never create more than one instance of any class or interface in your map and successfully register the new one. Your best hope would be for a HashMap(Class<?>,Set<Object>) to represent such a mapping.

(sigh) Ok, so you're about using Class as key in hashmap.

It's a kind of service locator pattern. We have one such map per TE instance. When TE is asked to return some behavior, which is a type, it searches for such mapping in that map using class it received as a key. If it finds some type, it returns that value. If not, it falls back to superclass behavior. So yes, there mapping from type to object is unique, but it's unique per TE instance. So I see no problem at all, since TE can't have two behavior implementors for the same type. It's not an event processing, where you can have several event handlers per event type.

 

Another thing, your idea represents breaking an object (Like a TE) into sides. But a TE normally requires and assumes all six sides belong to it. You'd get farther along by creating MicroTEs where each gets an isosceles pyramid shaped area of the block. Then you could make aggregates of these to fill a block.

TileEntiry myWierdBlock = new AggregateTE(MicroTE ob_up, MicroTE ob_down,
    MicroTE ob_north, MicroTE ob_south,
    MicroTE ob_east, MicroTE ob_west);

 

That would at least be a start.

You're going too far by talking about block geometry. My approach doesn't break TE into sides. It allows TE to behave differently on different sides. There's no break since behaviors aren't independent objects.

The pattern of having different behaviors on the sides of block is pretty common in Minecraft mods. You're checking if some TE can, at example, send or receive items from a certain side, or send/receive some power. And some sides just don't provide such a function.

To achieve this, mod authors usually add methods to interfaces like:

public int putItem(ItemStack stack, ForgeDirection direction);

Approach with sided behaviors would eliminate this need. Wanna your block work as inventory on a side? Just return inventory interface from there. Wanna check if the block is an inventory on a side? Just request behavior and check it for null.

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.

Announcements



×
×
  • Create New...

Important Information

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