rafradek / Mods

HomePage
Other
5 stars 5 forks source link

[Issue] Incompatibility with Dynamic Stealth? #47

Closed Secretname2009 closed 5 years ago

Secretname2009 commented 5 years ago

Im getting this crash randomly when the difficulty is set to allow the Medic to spawn, disabling Medic spawn completely fixes the random crash but i cant have a Medic around in the game because of this.

I tested this by spawning a Medic and by himself he didnt crash the game at all, i spawned a Heavy beside him and again nothing did happen, when i placed an enemy, both began attacking but a little after the game crashed giving me this error. I am suspecting Dynamic Stealth since i removed it from the pack once and it didnt crash with the same test above crash-2019-01-05-server.txt

Laike-Endaril commented 5 years ago

When I get a chance I will look at this over on my end (I'm the DS mod dev). I'll post here if I have any news.

Secretname2009 commented 5 years ago

Nice to see you trying to fix this. I will be available to help With this as both TF2 Stuff Mod and Dynamic Stealth goes quite well together, in fact the combination might be OP thanks to the instakill backstab the butterfly knife can do. This error only occurs when a Medic tries to look at a teammate to heal him but it happends while he gets attacked at the same time.

Laike-Endaril commented 5 years ago

I've confirmed that this is easy to reproduce using only TF2 Stuff and Dynamic Stealth, and that it does not happen if DS is removed. Also, the last usable code pointer in the stacktrace is pointing to this line in EntityAITasks:

for (EntityAITasks.EntityAITaskEntry entityaitasks$entityaitaskentry : this.taskEntries)

So I'm guessing a task is getting removed from taskEntries during this iteration. So far I've not been able to find any code in DS which would do this though.

Laike-Endaril commented 5 years ago

I've found the cause, and it's a 2 part issue, which makes sense in this case.

Now, if only TF2 is installed this is not an issue, because the Medic's setAttackTarget() method is not normally called during the medic's tasks.updateTasks() method.

This is where the DS part comes into play. Part of Dynamic Stealth's core uses an AI task to decide whether an entity's current attack target is valid, based on stealth checks. If it is not, it calls setAttackTarget(null).

And thus we have our issue. I'll see if I can rig a solution to this on my end, though I feel it may be a good idea not to remove task entries from within the setAttackTarget() method, as it may cause future issues with other mods.

Within 24 hours, I'll either have a new version of DS with a compatibility fix, or come back and say that I can't reasonably fix it from my end.

Laike-Endaril commented 5 years ago

Slight correction; while DS does call setAttackTarget(null) when the current target is not valid, I don't believe that's where the issue lies, as in the case of a null target, setCombatTask() does not seem to alter task entries.

However, in much the same way, in the same AI task in DS, there is a section which forces a target based on a threat system:

` //Threat > 0 and threatTarget != null...we have an existing target from before

if (AITargetEdit.isSuitableTarget(searcher, threatTarget)) { //Existing target's current position is known lastKnownPosition = threatTarget.getPosition(); clearSearchPath(); searcher.setAttackTarget(threatTarget); return false; } `

THIS is the DS-relevant part of the issue. Back to searching for a possible solution ~

Laike-Endaril commented 5 years ago

As of Dynamic Stealth version 026, I've applied a workaround by making medics bypass my threat system. It's not a perfect solution, but it's better than nothing.

I'd still suggest changing TF2 Stuff's code so that medics don't remove task entries from within their setAttackTarget() override, but that's not my choice to make. And by that token, @Secretname2009 I suggest you leave this issue open and allow the TF2 Stuff devs to close it if/when they feel it's appropriate, but again, that's just my suggestion.

My final note on this subject, to the TF2 Stuff devs; if you do happen to change the medic's setAttackTarget() method so that it doesn't remove task entries, please tell me so I can re-allow the medic in my threat system (so that it can use my search AI and whatnot), assuming it doesn't have any other conflicts when I do so ofc.

Secretname2009 commented 5 years ago

I will leave this open for sure, i suggest to you @Laike-Endaril adding a blacklist for entities that do cause this to prevent such issues in the future from other mods aswell if it is possible to make that a thing.

rafradek commented 5 years ago

Thank you for covering the issue, i've made some changes that should fix the incompatibility.

Laike-Endaril commented 5 years ago

@rafradek Nice! I will keep an eye out for the new version, and change my own code accordingly when it's released.

@Secretname2009 Yeah, I have entity-specific threat bypass config on my todo list, just hadn't gotten around to it yet...I'll probably add that today (unless something else comes up).

Secretname2009 commented 5 years ago

crash-2019-01-11_13.37.43-server.txt Update on this, i hit a crash regarding the Sentry Gun from TF2, it seems similar to previous problem. It happened instantly as i joined my own world. I saw the threat bypass whitelist worked with the Sentry Gun, i dont know what it tried to target but it certainly crashed because of that.

Laike-Endaril commented 5 years ago

Somewhat similar (I'm subscribed to this thread so I figured I'd take a look)

https://github.com/rafradek/Mods/blob/d7f41eb28111c39b941bf53430be17be57b30685/tf2/src/main/java/rafradek/TF2weapons/entity/ai/EntityAISentryAttack.java#L40

The task has its own target variable, which it checks for null, but then it calls shootBullet(), which uses getAttackTarget() for targeting, so what probably happened is...

  1. The sentry gun's default targeting was able to target something, but Dynamic Stealth rejected that target based on the stealth check and set the gun's attack target to null
  2. The ai task checked its own target variable for null, and it was not null, (even though the sentry's vanilla attack target was null)
  3. The shootBullet() method attempted to access a field or method within getAttackTarget() without checking whether the vanilla attack target was null first:

https://github.com/rafradek/Mods/blob/d7f41eb28111c39b941bf53430be17be57b30685/tf2/src/main/java/rafradek/TF2weapons/entity/building/EntitySentry.java#L277

In this particular case, the threat bypass be the better option, just because I don't know what will happen when my ai task tries to give a sentry gun a path to follow! (I'm assuming sentry guns are not supposed to move). So that's one more thing to consider if you decide to alter it @rafradek

rafradek commented 5 years ago

I simply removed the target field from sentry ai, this should fix the problem. Sentry guns have movement speed set to 0 so they can't move by move helper

Laike-Endaril commented 5 years ago

Nice, if sentry guns start moving around after your fix is released I'll take care of it on my end, because I think my mod may overwrite the speed atm so that would be my fault if they move

rafradek commented 5 years ago

This should be fixed now, I'm closing it

Laike-Endaril commented 5 years ago

Confirmed, the medic seems to work fine without threat bypass now.

The sentry gun doesn't crash either, though it seems to constantly lose target when not set to threat bypass mode. Sentry guns seem like something that should bypass the threat system anyway imo, but I might look into this sometime anyway and see what the cause is. For now I will add the sentry to the default threat bypass list.