OpenPerpetuum / PerpetuumServer

The Open Perpetuum Project's fork of the Perpetuum Standalone Server
https://openperpetuum.com
Other
44 stars 21 forks source link

NPC range check should not use miner/harvester ranges #402

Open MikeJeffers opened 2 years ago

MikeJeffers commented 2 years ago

This query/computation : https://github.com/OpenPerpetuum/PerpetuumServer/blob/Development/src/Perpetuum/Zones/NpcSystem/Npc.cs#L1301-L1304 includes miner/harvester/RSA/remote reps etc. and that is likely not where the npc needs to limit its range to. Figure out a better way to filter to the desired set of offensive modules and compute the min range (instead of just active and IsRanged)

Ramit110 commented 1 year ago

Not the best implementation but it does work for any weapon that uses WeaponModule or is a subclass of WeaponModule.

This would break if the class is in someway refactored or modified and I still need to get my local branch working to fully test this.

Ramit110 commented 1 year ago

Thinking about the current implementation the range of neutralisers/vampires or other non-weapon combat modules won't be included so this code likely won't be the best, I guess I can:

MikeJeffers commented 1 year ago

Yes, good observation. It might instead be good to do a negation of some known non-combative ranged modules that some npcs are equipped with. I could see it working either way with your 2 approaches. I think the property might be good, more efficient to filter on, and could simply be override in the subclasses that you wish to exclude from this range-computation. But wouldn't worry too much about optimizing the filter as it is cached per npc instance. Perhaps the property is more maintainable? Just might need a note that it is used for precisely determining this npc property for those implementing new modules.

andrey-at-home commented 1 year ago

I suggest using the OffensiveModule property for filtering in this case.

Ramit110 commented 1 year ago

Just need to test this now

andrey-at-home commented 1 year ago

I tested a new version of the filter on industrial NPCs:

PS: Scarabs do not have weapons, only electronic warfare modules., They still try to keep a line of sight with the player.

Ramit110 commented 1 year ago

Would that last part be considered a bug? Otherwise it looks much better!

andrey-at-home commented 1 year ago

It doesn't seem to be a bug, it's a feature.