Minimum information needed:
Which shard is affected?
All
What happened?
When utilizing an some functions, such as attack(), in a room with safe mode, misleading values can be returned (example: attack returns -12 ERR_NO_BODYPART)
What should have happened?
Up for discussion, see below
How can we reproduce this?
Have a creep attempt to attack a hostile structure/creep in a room with safeMode active
While I was going to submit a PR changing the return values for functions like attack() and rangedAttack(), in my opinion rangedMassAttack() raises a problem.
When utilizing attack functions like these in a room that is not yours, and has safe mode active, the return value is ERR_NO_BODYPART.
Engine /src/game/creeps.js #604-605
if(this.room.controller && !this.room.controller.my && this.room.controller.safeMode) {
return C.ERR_NO_BODYPART;
When doing something like attempting to withdraw(), the return code is ERR_NOT_OWNER when you were never the owner of the targeted container!
Engine /src/game/creeps.js #528-529
if(this.room.controller && !this.room.controller.my && this.room.controller.safeMode) {
return C.ERR_NOT_OWNER;
I feel this is extremely misleading. Granted you should (ideally) not be utilizing these methods in a room with safe mode in the first place (often saves CPU on find checks/pathfinding to avoid these sections entirely, and keeps creeps from exploring into rooms they have no purpose in), but I don't believe it is appropriate to design the API with idealized code in mind. New players may utilize these functions and be confused by this return code (as happened in the Discord today with attack()) I spent a bit of time trying to help this player identify the cause, only for their suggestion of safe mode being active to finally lead me to this issue.
In my opinion, the ideal solution is a new error value: ERR_SAFEMODE_ACTIVE or similar, but ERR_INVALID_TARGET is also acceptable. My reasoning for these two are as follows:
Argument for ERR_SAFEMODE_ACTIVE
I originally wanted to suggest a change to ERR_INVALID_TARGET, but this raises two issues.
Less importantly, it does not make a distinction that safe mode is active. A target that would otherwise be a valid target (eg. a hostile spawn) can become an invalid target the tick that safe mode is activated. This might(?) be confusing for a new developer, but I find this less problematic as players have the responsibility to have an understanding of the game mechanics.
More importantly: ERR_INVALID_TARGET would be an extremely odd return code for functions like rangedMassAttack() which have no target.
Especially for new players, ERR_SAFEMODE_ACTIVE or similar would be extremely clear as to the cause of failure for that action. While these actions are often executing with invalid targets, some actions now (and possibly in the future) are not targeting and thus would not
Argument for ERR_INVALID_TARGET
While less specialized, this does not require the adding of a new constant. It is much more clear than ERR_NO_BODYPART, as that is what is happening: whatever you are doing (even rangedMassAttack()), the target is not valid (since you cannot target anything within a safemode room for these actions.)
There is even precedent for this. When attempting to move into a space where any creep is present while safe mode is active the game returns ERR_INVALID_TARGET
Engine /src/game/creeps.js #799-807
if(_.contains(C.OBSTACLE_OBJECT_TYPES, target.structureType)) {
if(_.any(objectsInTile)) {
return C.ERR_INVALID_TARGET;
}
const blockingCreeps = (this.room.controller && this.room.controller.my && this.room.controller.safeMode) ? myCreepsInTile : creepsInTile;
if(_.any(blockingCreeps)) {
return C.ERR_INVALID_TARGET;
}
}
Power creeps also return things such as ERR_INVALID_ARGS or ERR_INVALID_TARGET for certain actions when safe mode is active in the room. usePower(), if it functioned similar to things like attack() on creeps, would return ERR_NO_BODYPART if we were remaining consistent with our return codes.
I hope one of the two will be considered. It is not a pressing change, but either solution should be a minor change to the engine code that would be much clearer and more accurate in my opinion.