Suggestion for a few small changes

[ZDoctor]

I'll keep this short, the EntityLivingBase class has a protected variable protected float lastDamage. Is there a reason this is protected? I used this to calculate what the previous health was by adding this value to the current health. Since I wasn't using a class that extended this class, I used reflection to get this value. On that note, maybe even a lastHealth variable would be nice as well.

 

Another thing I noticed in that same class that anything that has to do with how potions are only handled on the server. For example, addPotionEffect calls onNewPotionEffect that will not apply the attribute modifiers on the client side as there is a check to see if the world is not remote. Same thing with updatePotionEffects. There is a check to ignore the changes and not add it to the active portion. But when the world is reloaded, the potions are then added to the client. Because it is not allowed to do anything regarding potions, the inactive potions are left in the entity potion map. This isn't a problem for the local player, but other entities will still return true for an expired potion effect (i.e. poison) until the next reload. This is a problem if you want to add a client-side gui of some kind. that displays the effects. I would suggest removing the check from the class.

[Animefan8888]

10 minutes ago, ZDoctor said:

I would suggest removing the check from the class.

This will not happen, this is to ensure that potion effects are not changeable client side, for obvious reasons(it can and will be exploited).

 

12 minutes ago, ZDoctor said:

Is there a reason this is protected?

It is protected because Mojang had no use for it to be public, aka it wasn't intended to be used outside of an Entity.

14 minutes ago, ZDoctor said:

Since I wasn't using a class that extended this class, I used reflection to get this value.

What is it that you are getting this for?

15 minutes ago, ZDoctor said:

On that note, maybe even a lastHealth variable would be nice as well. 

This will not happen as this requires too much editing for a feature that will not really be used for much.

 

 

[ZDoctor]

10 hours ago, Animefan8888 said:

It is protected because Mojang had no use for it to be public, aka it wasn't intended to be used outside of an Entity.

What is it that you are getting this for?

 

I am getting the last damage to calculate what the last health was by adding the current health and last damage together. Could not a getter for this combination be added?

 

10 hours ago, Animefan8888 said:

This will not happen, this is to ensure that potion effects are not changeable client side, for obvious reasons(it can and will be exploited).

 

Here is what I did to prevent that abuse. I would be more than happy to add the relevant code if people think that it is a simple enough solution.

 

First I created a Potion Watcher (data parameter/watcher):

public static final DataParameter<NBTTagCompound> WATCHER_POTION_EFFECTS = EntityDataManager.createKey(EntittLivingBase.class, DataSerializers.COMPOUND_TAG);

Then overrode the entity init (to implement you would just add to the method):

@Override
protected void entityInit() {
	super.entityInit();
	this.dataManager.register(WATCHER_POTION_EFFECTS, new NBTTagCompound());
}

Then I overrode the notifyDataManagerChange and checked to see if the world was a client and if the parameter changed was the potion effect watcher:

@Override
public void notifyDataManagerChange(DataParameter<?> key) {
	super.notifyDataManagerChange(key);
	if (world.isRemote && WATCHER_POTION_EFFECTS.equals(key)) {
		NBTTagList nbttaglist = this.dataManager.get(WATCHER_POTION_EFFECTS).getTagList("ActiveEffects", 10);
		activePotionsMap.clear();
		for (int i = 0; i < nbttaglist.tagCount(); ++i) {
			NBTTagCompound nbttagcompound = nbttaglist.getCompoundTagAt(i);
			PotionEffect potioneffect = PotionEffect.readCustomPotionEffectFromNBT(nbttagcompound);
			if (potioneffect != null) {
				addPotionEffect(potioneffect);
			}
		}
	}
}

Added this to the resetPotionEffectMetadata:

@Override
protected void resetPotionEffectMetadata() {
	super.resetPotionEffectMetadata();
	// So the data will sync
	this.dataManager.set(WATCHER_POTION_EFFECTS, new NBTTagCompound());
}

 

Added this to the updatePotionMetadata, making sure the server was the only one handling this kind of change for active potion effects:

@Override
protected void updatePotionMetadata() {
	super.updatePotionMetadata();
	// So only server can send updates, would cause recursion otherwise
	// Would potentially allow the client to add effects, and we wouldn't want that
	if (world.isRemote)
		return;

	NBTTagList nbttaglist = new NBTTagList();

	for (PotionEffect potioneffect : getActivePotionMap().values()) {
		nbttaglist.appendTag(potioneffect.writeCustomPotionEffectToNBT(new NBTTagCompound()));
	}

	NBTTagCompound compound = new NBTTagCompound();
	compound.setTag("ActiveEffects", nbttaglist);

	this.dataManager.set(WATCHER_POTION_EFFECTS, compound);
}

 

And that's it. It leaves the rest of the code as it is only added a couple dozen lines and ensures that only the server can add potion effects while making sure the client is aware of the changes. I think because potion effects are stuck on the client after reload until next reload that would be considered a memory leak and this will ensure that doesn't happen, but I could be wrong about the terminology. I've tested this on my own mod and it works as intended. Is there something I am missing?

Edited September 13, 2018 by ZDoctor
Fixed code

[Animefan8888]

1 minute ago, ZDoctor said:

Could not a getter for this combination be added?

It could, but why you can do a simple addition yourself.

2 minutes ago, ZDoctor said:

I am getting the last damage to calculate what the last health was by adding the current health and last damage together.

I meant why do you need the health prior to the last damage.

 

5 minutes ago, ZDoctor said:

Is there something I am missing?

I'm not sure I understand why you are going about doing this. What is causing a problem about the vanilla implementation.

[ZDoctor]

1 hour ago, Animefan8888 said:

It could, but why you can do a simple addition yourself.

 

It's not that the addition that is making it difficult, but not being able to easily access the variable in the first place.

1 hour ago, Animefan8888 said:

I meant why do you need the health prior to the last damage.

 

People may find a variety of cases where they have the need for the last health of the entity and not know of a way to get it. For my case, I was making a simple Damage Indicator mod that when the health changed it displayed a certain effect if the health went down.

1 hour ago, Animefan8888 said:

I'm not sure I understand why you are going about doing this. What is causing a problem about the vanilla implementation.

2

 

As I said before, what is causing my problem is that the client is unaware of when potion effects are initially active on an entity.

 

Here are some pictures to help demonstrate my point.

Here are two entities after they are initially poisoned:

 

Their hearts remain red. Then after leaving and reentering the world:

 

Then after the effect expires (you can tell because of lack of particles):

They are no longer poisoned, but they don't know it yet.

And it stays like that until the next reload:

As a technical note, the PotionEffect onUpdate is still called every tick like an active potion would, but does nothing but waste resources.

 

That is the problem I have with the vanilla version that I think can be easily solved with the solution that I added above.

Edited September 13, 2018 by ZDoctor
Better pictures

[Cadiboo]

Where are the particles spawned? Wouldn't whatever class that is in know what the potion effect is?