Astral Sorcery

Astral Sorcery

63M Downloads

Discidia using IMob check limits its value

smilodon79 opened this issue ยท 10 comments

commented

I don't know since when Discidia started checking for mob type to be IMob. The result is, it doesn't kill any passive or modded mobs (pigs, cows, sheep, thaumcraft wisps, eldritch guardians, vampirism vampires, etc.). Request that check to be taken out of CEffectDiscidia.java or a configuration option introduced, to be able to list mobs that are white/black listed.

Offending line: (entity) -> !entity.isDead && !(entity instanceof EntityTechnicalAmbient) && entity instanceof IMob);

commented

b8a8cc5

That is the only recent change, which isn't published yet. Which is what will be going live next update.

commented

They should be overriding the ai, in that case.

commented

@Doomgull that's exactly what Hellfire and I already discussed. Nobody debates that mod authors should ideally be implementing IMob. But since they don't, it leaves us with figuring out what to do, or leave discidia toothless, like it currently is. This issue is really a follow-up of Defect #1141 , and shows that the fix provided by the bug creator doesn't entirely fix the issue.

I'd request we remove the "instanceof IMob" and make discidia the true damaging ritual that it should be (less ideal), or have a whitelist and blacklist to add or remove entity types to the ritual's affected mobs. I know it's a hacky solution, but it's cleaner than removing the IMob check entirely, or leaving discidia toothless.

commented

"instead of extending IMob, end up extending EntityLiving"
"which as a developer, I'd choose myself"

So, you should know the difference between an interface and a class.

Edit: on top of that, IMob is just a marker interface.

commented

I've already reviewed and tested the change you indicated (from Github). Since you have retained the condition I indicated in lines 58 & 59: entity instanceof IMob, it still only ever kills IMobs (vanilla hostile mobs). That's why I only metioned that offending line in my original bug, and not every reference of IMob instead of IEntityLiving.

commented

It's meant to kill hostile entities. Which in vanilla all implement the IMob interface. If a mod does not do this, it either intentionally wants to avoid it triggering hostile-entity related mechanics or forgot to do so.

commented

@HellFirePvP , actually, a ton of mods need specialized AI and, admittedly, instead of extending IMob, end up extending EntityLiving instead and write their customized code. The net result is Discidia ends up being toothless.

While I completely agree with your puristic approach (which as a developer, I'd choose myself), may I please recommend a compromise in the form of a configuration to whitelist and/or blacklist entities with maybe wildcard support? That would help server admins like me to get a more nuanced control on how useful I should retain the ritual, among others.

commented

@HellFirePvP low blow. Fair point about implementing the interface vs extending the base class. But nothing in your comment addresses my concern or my workaround. Ultimately, it's your prerogative to decide if you want to leave things be and hope discidia to become relevant when the other mob authors choose to follow better rigor, but I'd hope you at least consider making the ritual stay relevant.

commented

I chose to go check for IMob because all vanilla monsters - hostile entities - do so.
If a mod decides to not mark its hostile entities IMob it's that mod's choice to forgo vanilla minecraft concepts by not doing so, it's their choice. If they do implement IMob on their hostile entity, Discidia will pick up on the mob and do damage to it.

commented