Missing equipment after combat

[Eric Walch]

On the other hand both Target Autolock and Target Reticle Sensitive appear to have been deliberately scripted by their authors (Thargoid and Eric respectively) to be removed when Scanner Targeting Enhancement is damaged.

The main reason to do it that way is to reduce the amount of equipment in the equipment list. My first release was in the form of normal equipment. But at that time the equipment list could not be wrapped and would fall of the bottom of the screen when it became to long. I had several request to not add new equipment, so I decided to make it part of other equipment. The "Scanner Targeting Enhancement" in this case .

I now see it as an expensive modification on the "Scanner Targeting Enhancement", and when that breaks, the modification is also destroyed in the process.

Back to the original problem. I just committed a version r4598 were the condition check is skipped for equipment damaged in combat.

When damaged, equipment is always completely removed and than replaced by a version with "_DAMAGED" appended. In the past that was a fully independent equipment because of the different name. And for that damaged equipment no conditions existed and the code would always award the 'new' equipment.
But, at some point in the past, the damaged version became the original with a special flag set. The code is now fully non-logic and probably should just switch that flag, instead of removing and awarding stuff. However, having a feature lock it is better to stay away from fully refactoring and going for the simple solution and just skip the conditions check.

For the JS 'awardEquipment' method it would also be easy to add a second [optional] boolean parameter for checking the condition. Default would be yes, so existing commands won't change. That change needs the opinion of other dev members: if that would be a new feature or just correcting an incomplete implementation of the old method.
(It would be a trivial change that I already made and tested in my personal oolite copy, but I am not sure if it is correct to allow scripts to bypass the conditions, other than repairing already existing equipment.)

[Eric Walch]

This will have a big impact on repair bots/equipment as well - it makes items only repairable when the conditions at the time are correct to be able to buy the item from the station too. So either the conditions should only be checked for the purchase of equipment (and not when it breaks or gets its status changed back to OK from damaged) or possibly a new JS command variant of awardEquipment and/or setEquipmentStatus that explicitly ignored conditions and forces the change.

For 'setEquipmentStatus' I think that that command should skip the validation also because the ship already has the equipment.

[Eric Walch]

For 'setEquipmentStatus' I think that that command should skip the validation also because the ship already has the equipment.

On second thought, I am not sure anymore if it always should be skipped. As it is now it is wrong, because when you set equipment to DAMAGED, the equipment is completely removed. Setting to DAMAGED should work when conditions are not met. Setting it back to working is a whole other mater that probably should respect the conditions. That probably should do nothing, and not remove the damaged one as happens now.

EDIT:
I now made the change:
Setting to DAMAGED always works, no mater what the conditions are
Setting to OK only works with correct conditions, but damaged equipment is no longer lost when the repair fails.

[Capt. Murphy]

Hi Eric,

If I've a understand your change correctly this could still result is disappearing equipment for items such as the escort license where the author does not intend for them to be damageable and uses setEquipmentStatus(equipment,"EQUIPMENT_OK") from a world-script as instantaneous repair and the conditions are not met.

Edit - OK so it won't disappear - but will remain as damaged....mmmm....there is no other mechanism to make OXP equipment appear in the manifest and not be damageable. means extra scripting to work around this...

Similarly disappearing equipment will continue to be an occasional issue for repair-bots and similar when it uses setEquipmentStatus(equipment,"EQUIPMENT_OK") and the equipment item in question has conditions which are not met.

In light of the edit to your post above.

In short I think it remains a bug if the validation/condition checking is invoked for a setEquipmentStatus(equipment,"EQUIPMENT_OK") operation.

[Thargoid]

For 'setEquipmentStatus' I think that that command should skip the validation also because the ship already has the equipment.

On second thought, I am not sure anymore if it always should be skipped. As it is now it is wrong, because when you set equipment to DAMAGED, the equipment is completely removed. Setting to DAMAGED should work when conditions are not met. Setting it back to working is a whole other mater that probably should respect the conditions. That probably should do nothing, and not remove the damaged one as happens now.

EDIT:
I now made the change:
Setting to DAMAGED always works, no mater what the conditions are
Setting to OK only works with correct conditions, but damaged equipment is no longer lost when the repair fails.

That will potentially break in-flight repairs - they will only happen in systems where the conditions happen to be met. That to me makes no logical sense. For me not being able to set an existing damaged piece of equipment's status back to OK by script when in-flight just depending on where you happen to be flying would still be a bug.

I would agree with your original thought on this, that awardEquipment (fresh award of an item you don't already have) should check but setEquipmentStatus should not check. Either that or only have the check made when docked. My thinking is in line with Capt Murphy's above.

[CommonSenseOTB]

I'm starting to become concerned here guys. All my animated huds use equipment to activate pieces on a frame by frame basis and equipment that is damaged that I don't want to be damaged gets repaired instantly by setting the equipment status to "OK". Whatever you decide to do must allow scripters to unconditionally set the "DAMAGED" status to "OK" again. It is required for the operation of animated huds that use equipment activation in the hud.plist and therefore this compatibility must be upheld. The same goes for awardEquipment. You must implement a solution that does not break compatibility with my animated huds and other oxps. Perhaps the real problem lies with how Target Autolock and Target Reticle Sensitive work. Look at those first and if they need to be reworked then so be it.

[Thargoid]

TAP and reticle sensitive can now be written another way, as there is now a flag to make equipment not list on the equipment screen (and so by default be undamagable). That is not the fundamental issue here, the problem is being able to in-flight (or by script, to put it another way) change equipment status.

If your HUD changes equipment in-flight but that equipment does not have any requirements or conditions attached to it then there will be no issue for you. But if the relied-upon equipment does then you will have the issues mentioned.

For me if setEquipmentStatus makes any kind of check on requirements or conditions then for me the bug remains in place and is not resolved.

[Capt. Murphy]

Apologies for my slightly confused lunchtime post - I think Eric must have edited just before I originally hit reply.

CSOTB - What Thargoid said - you've only got something to worry about if your "EQ_"'s have conditions set like "requires_clean" etc. I had a quick peek at the NumericHUD equipment.plist and they haven't, so this bug (and it's fix/partial fix depending on your point of view) shouldn't have any impact on your HUDs.

Thargoid - I'm not sure if the "visible" flag stops equipment being damaged? There is a hardcoded canBeDamaged flag that defaults to YES and isn't valid in equipment.plist (is set to NO for the Passenger Berth, Trumbles, Cargo Bay Extension and Mines/Missiles), but that's not much use for OXPers.

[Eric Walch]

Thargoid - I'm not sure if the "visible" flag stops equipment being damaged?

I am sure that it won't protect equipment. It only has influence on showing up in the inventory at the F5F5 screen. That might give a problem as the player won't be aware that it is broken. But it would show at the buy-equipment screen when damaged and it was sellable equipment.

I have been brainstorming what could go wrong when skipping the conditions check altogether. For the requires_clean it is clear, checking feels as a bug there. I was worried that some conditions might spoil it, so I evaluated all possible conditions. (there were quite a lot new once added recently. In the equipmentInfo page they were all listed, but not on the equipment.plist page itself. (And the syntax is different there) So, I updated that wiki page a bit.

I ended up brainstorming with the conclusion that I also can't see a reason to keep the condition checks in for changing an existing equipmentStatus. The player already has it anyhow. I leave it a few days and when no objections are received, I commit it in a form without any condition checks.

[Thargoid]

Perfect. My memory about the non-visible equipment being undamagable came from when it was introduced, but may well be wrong.

And as for conditions, I was also thinking of the explicit conditions that you can set for equipment (mission variable requirements, specific systems, that kind of thing) as well as the equipment.plist ones such as legal status etc. Those are the more subtle ones that will also trip things up on repair/status restoration to OK.

Thanks Eric.

[Capt. Murphy]

Good call Eric!

[CommonSenseOTB]

Sounds good Eric. While we're on the subject, the "canBeDamaged" would really be useful to oxp's particularly if there is a large amount of equipment that the author doesn't want damaged, ever. In the equipment list for my animated huds they all use "visible" to prevent seeing a hundred odd pieces of equipment in the ship's inventory and if "canBeDamaged" were useable by the oxps I would have used that as well and would not need to have any equipment damaged checks for them as well. If it is ever available to use by oxp's in the future it would definately be useful.

Thankyou Eric.

[Eric Walch]

Searching and evaluating all occasions inside the code, I found another bug with validation. Currently validation is by default set to true and all occasions of awarding equipment do a validation, even when installing equipment on loading a saved game. When the player had it on saving, it should be there on loading as well. I think the easiest solution is make false the default. There is only one place were validation is needed and that is in the JS command to prevent adding incompatible equipment. Even at the buy-equipment screen the addEquipment is inside a loop that already did a validation.

I just tested an equipment type with condition "requires_not_clean". That equipment is lost on loading a saved game. That is because bounty is set after equipment is loaded. Apparently nobody used that condition in equipment
That is also the only reason why "requires_clean" gives no problem with a save game with bounty: On installing equipment while loading a saved game, the player bounty was still listed as zero.

[Eric Walch]

I just tested an equipment type with condition "requires_not_clean". That equipment is lost on loading a saved game. That is because bounty is set after equipment is loaded. Apparently nobody used that condition in equipment

The problem of restoring a saved game is more complicated than I first thought. Restoring happens in "addEquipmentFromCollection". There all equipment was already added without validation in a first pass. But than in a second pass all equipment is validated again to be able to remove incompatible equipment. Equipment that fails is removed again. And because player.bounty is zero at that moment, any equipment that "requires_not_clean" is always removed.
But when equipment has in its condition-array some conditions that are not met, that is no problem as the condition-array is ignored in this test. Only the flags are evaluated at this point. The conditions-array is apparently only used on the buying screen.

That probably means we need two versions of that check. One normal and one with a reduced set while loading a game..... Is working now, will test further.

[Eric Walch]

Now things are sorted out in r4599 and should be in next nighty.

- setEquipmentStatus() no longer looks at the various conditions.

- only part of the conditions are now checked on startup. In principle things like bounty should be the same on loading and saving anyhow. Only when loading a game with an other oxp set installed can make is necessary to validate dependencies and incompatibilities again.

A bug still left is that preinstalled equipment on the buy screen is still validated. But that should already be validated before showing up. Currently there is only a check for incompatible equipment so you won't get a ship pre-fitted with two incompatible items. But there is no check for dependencies. That means when an oxp with ships defines for example the "EQ_ORE_PROCESSOR" as optional equipment in shipyard.plist, it can happen that you get a ship with that equipment for sale. But as it is dependent on the fuelScoop, the equipment will not be there after buying when there was not also a fuelScoop pre-fitted on the ship.
But I can't remember any oxp that adds optional equipment from outside the core set, so it won't be a big problem. For now you should just avoid adding such dependent equipment from other oxps in your shipyard. (If one of the others devs does want it fixed they can look at it)