Issue Data
|
Issue #31604: Assigning actors to beds does not clear their old bed assignment
I doubt I'm the first person to've encountered this, but I didn't find a prior report, perhaps due to unfamiliarity with the search tools.
In any case, an actor being assigned to a bed through a workshop menu is intended to clear ownership of any other beds they may have already owned. However, this isn't happening and you can easily end up assigning the same actor to a half-dozen different beds. The problem as far as I have been able to troubleshoot it has several components, all stemming from one fairly short bit of code in WorkshopParentScript's AssignActorToObject function. I quote the relevant piece here: function AssignActorToObject(WorkshopNPCScript assignedActor, WorkshopObjectScript assignedObject, bool bResetMode = false, bool bAddActorCheck = true, bool bUpdateObjectArray = true) ;/ assorted remarked-out code ----------------------------------------------------------------------------------------------------------------------------------------- UFO4P 2.0.4 Bug #24312: Performance optimizations required substantial modifications to this function. In order to maintain legibility, the code has been rewritten. Any comments on modifications prior to UFO4P 2.0.4 (except for official patch notes) have been left out (they are still preserved in the commented out code above though). A summary of the modifications is included in the comment on the ResetWorkshop function. ;----------------------------------------------------------------------------------------------------------------------------------------- /; wsTrace(" AssignActorToObject: actor = " + assignedActor + ", object = " + assignedObject + ", resetMode = " + bResetMode) int workshopID = assignedObject.workshopID WorkshopScript workshopRef if workshopID >= 0 workshopRef = GetWorkshop (workshopID) else wsTrace(" AssignActorToObject: ERROR - object " + assignedObject + " has no valid workshop ID. Returning ...", 2) return endif if bAddActorCheck AddActorToWorkshop (assignedActor, workshopRef, bResetMode) endif WorkshopNPCScript previousOwner = assignedObject.GetAssignedActor() bool bAlreadyAssigned = (previousOwner == assignedActor) if assignedObject.IsBed() bool UFO4P_IsRobot = (assignedActor.GetBaseValue(WorkshopRatings[WorkshopRatingPopulationRobots].resourceValue) > 0) if bAlreadyAssigned wsTrace(" Bed - Already assigned to this actor" if UFO4P_IsRobot wsTrace(" Actor is robot - clearing ownership" assignedObject.AssignActor (none) UFO4P_AddUnassignedBedToArray (assignedObject) endif ;UFO4P 2.0.4 Bug #24408: don't try to assign beds to robots: elseif UFO4P_IsRobot wsTrace(" Bed - Actor is robot. Ignoring ..." else wsTrace(" Bed - Clearing ownership of any other bed" ;No need to run this if assignedActor is in the UFO4P_ActorsWithoutBeds array: ;Note: after a workshop loads, this function will not run until ResetWorkshop starts looping through the resource object ;arrays, and at this point, the UFO4P_ActorsWithoutBeds array is already up to date. if UFO4P_ActorsWithoutBeds && UFO4P_ActorsWithoutBeds.Find (assignedActor) < 0 ObjectReference[] WorkshopBeds = GetBeds (workshopRef) int countBeds = WorkshopBeds.Length bool ExitLoop = false int i = 0 while i < countBeds && ExitLoop == false WorkshopObjectScript theBed = WorkshopBeds[i] as WorkshopObjectScript if theBed && theBed.GetActorRefOwner() == assignedActor theBed.AssignActor (none) UFO4P_AddUnassignedBedToArray (theBed) ExitLoop = true endif i += 1 endWhile endif wsTrace(" Assigning bed " + assignedObject + " to " + assignedActor) assignedObject.AssignActor (assignedActor) ;UFO4P 2.0.6 Bug #25483: added the following lines: ;If there was no previous owber, this bed must have been in the unassigned beds array, so it should be removed now. if previousOwner == none UFO4P_RemoveFromUnassignedBedsArray (assignedObject) endif endif elseif assignedObject.HasKeyword (WorkshopWorkObject) ;rest of function EndFunction First and most obviously, while the trace says the code is intended to clear 'any' other bed, because of the ExitLoop boolean causing a break in the while loop, it will only ever remove *one* other bed; the code silently assumes that no more than one bed can be owned at a time. This true only if everything is working correctly, which it isn't. I don't think the speed savings here are worth the loss of robustness and ability to recover. Secondly, the check "if UFO4P_ActorsWithoutBeds && UFO4P_ActorsWithoutBeds.Find (assignedActor) < 0" ahead of the while loop will return an unexpected false, leading to the unassignment loop not being entered. The cause of this is the UFO4_ActorsWithoutBeds array being a none array. However, the check here appears to be redundant; the followup Find() call will return -1 if called on a none array, an empty array, or an array that does not contain the searched-for value. Replacing the if statement with simply "UFO4P_ActorsWithoutBeds.Find (assignedActor) < 0" allowed the unassignment loop to proceed as expected. I am not sure at this point as to why the array is a none array. Best guess is that, as per comments elsewhere in WorkshopParentScript, the game engine will convert 0-length arrays to none arrays. Since the array should be a zero-length one in my testing, as all actors have assigned beds (confirmed via various trace commands), this could be occurring. Thirdly, as far as I can tell, there's no provision in this code to update the UFO4P_ActorsWithoutBeds array, either when assigning one to an actor (who should then be removed from that array) or when a bed's prior owner is unassigned from it (who should then be *added* to that array). While these should be rebuilt on a ResetWorkshop() call, in the interim -- if, for example, assigning multiple actors in a row -- the array will get progressively out of date. I may be missing something, but I *believe* that the UFO4P_ActorsWithoutBeds array should be updated in this code just as the UFO4P_UnassignedBeds array is. I took a swing at revising the code: wsTrace(" Bed - Clearing ownership of any other bed" ;No need to run this if assignedActor is in the UFO4P_ActorsWithoutBeds array: ;Note: after a workshop loads, this function will not run until ResetWorkshop starts looping through the resource object ;arrays, and at this point, the UFO4P_ActorsWithoutBeds array is already up to date. if UFO4P_ActorsWithoutBeds.Find (assignedActor) < 0 ObjectReference[] WorkshopBeds = GetBeds (workshopRef) int countBeds = WorkshopBeds.Length int i = 0 while i < countBeds WorkshopObjectScript theBed = WorkshopBeds[i] as WorkshopObjectScript if theBed && theBed.GetActorRefOwner() == assignedActor theBed.AssignActor (none) UFO4P_AddUnassignedBedToArray (theBed) endif i += 1 endWhile endif wsTrace(" Assigning bed " + assignedObject + " to " + assignedActor) assignedObject.AssignActor (assignedActor) UFO4P_ActorsWithoutBeds.Remove(UFO4P_ActorsWithoutBeds.Find(assignedActor)) ;UFO4P 2.0.6 Bug #25483: added the following lines: ;If there was no previous owber, this bed must have been in the unassigned beds array, so it should be removed now. if previousOwner == none UFO4P_RemoveFromUnassignedBedsArray (assignedObject) else UFO4P_AddToActorsWithoutBedsArray(previousOwner) endif This appears to work, and will assign and unassign actors from beds, but I have not yet tested robustly for any other issues that might come up -- in particular I'm uncertain about the updating of the UFO4P_ActorsWithoutBeds array. |
This has been an issue I have experienced a lot. You can literally assign them 20 beds and they'll all remain taken even if there is just one settler at a settlement. Would be great to get to the bottom of this.
Couple of notes: After playing more with my code above running I spotted two issues with it.
First, there's no need, and in fact it causes issues, to add the actor to the ActorsWithoutBeds array if they are unassigned from a bed.
In the case where an actor owned two or more beds, and was then unassigned from one (but not all!) of them, adding the actor to that array would be incorrect. This arises when another actor is assigned to an owned bed; only that one specific bed has its ownership cleared. In this case you do still want to run the full unassignment loop if the actor with multiple beds is then assigned to another one.
If, on the other hand, an actor isn't added even though it should have been, it simply means that the ownership unassignment loop will run unnecessarily, but harmlessly, if the actor has no bed. It does mean the actor will not be autoassigned with the TryToAssignBed() calls outside of the ResetWorkshop() function, but that will be cleared on the next reset workshop and doesn't otherwise cause data integrity issues.
Thus, the lines in my fix above of:
The removal of the bed from UFO4P_UnassignedBedsArray is also now protected by this conditional.
For similar reasons, the addition of the bed to the UFO4PP_UnassignedBedsArray should also be protected by a conditional check, so:
should become
Alternatively, the condition checks could be placed directly in the UFO4P_AddUnassignedBedToArray() and UFO4P_RemoveFromUnassignedBedsArray() functions, which would protect them against incorrect data coming in from other potential calls. A similar modification might be useful for the work objects array helper functions as well. It might also be a good idea to create a UFO4P_RemoveFromActorsWithoutBedsArray() function, analogous to the ones for beds and work objects, to encapsulate logging and basic 'these things exist' checking.
First, there's no need, and in fact it causes issues, to add the actor to the ActorsWithoutBeds array if they are unassigned from a bed.
In the case where an actor owned two or more beds, and was then unassigned from one (but not all!) of them, adding the actor to that array would be incorrect. This arises when another actor is assigned to an owned bed; only that one specific bed has its ownership cleared. In this case you do still want to run the full unassignment loop if the actor with multiple beds is then assigned to another one.
If, on the other hand, an actor isn't added even though it should have been, it simply means that the ownership unassignment loop will run unnecessarily, but harmlessly, if the actor has no bed. It does mean the actor will not be autoassigned with the TryToAssignBed() calls outside of the ResetWorkshop() function, but that will be cleared on the next reset workshop and doesn't otherwise cause data integrity issues.
Thus, the lines in my fix above of:
else UFO4P_AddToActorsWithoutBedsArray(previousOwner) [/code] are unnecessary and can be removed. Secondly, it can happen that assigning an owner to a bed can fail. I am not sure of the precise reason, but I've notice it happening with pre-placed beds in particular. As such, to avoid inappropriately removing an actor from ActorsWithoutBeds, there should be a conditional check directly following the AssignActor() call to make sure that the actor was indeed made the owner of the bed in question before removing them. While the actor must be removed from the array if they have a bed (otherwise subsequent assignments to other beds won't clear the first one) removing them from the array unnecessarily will interfere with TryToAssignBeds() calls executed outside of a ResetWorkshop() call -- the actor will be assigned a bed even though it already has one. The call inside ResetWorkshop() itself is fine, since ResetWorkshop() rebuilds the arrays from scratch to begin with. Therefore, in my fix, [codebox] assignedObject.AssignActor (assignedActor) UFO4P_ActorsWithoutBeds.Remove(UFO4P_ActorsWithoutBeds.Find(assignedActor)) ;UFO4P 2.0.6 Bug #25483: added the following lines: ;If there was no previous owber, this bed must have been in the unassigned beds array, so it should be removed now. if previousOwner == none UFO4P_RemoveFromUnassignedBedsArray (assignedObject) else UFO4P_AddToActorsWithoutBedsArray(previousOwner) endif [/code] should instead be: [codebox] if assignedObject.GetActorRefOwner() == assignedActor UFO4P_ActorsWithoutBeds.Remove(UFO4P_ActorsWithoutBeds.Find(assignedActor)) ;UFO4P 2.0.6 Bug #25483: added the following lines: ;If there was no previous owber, this bed must have been in the unassigned beds array, so it should be removed now. if previousOwner == none UFO4P_RemoveFromUnassignedBedsArray (assignedObject) endif endif
The removal of the bed from UFO4P_UnassignedBedsArray is also now protected by this conditional.
For similar reasons, the addition of the bed to the UFO4PP_UnassignedBedsArray should also be protected by a conditional check, so:
while i < countBeds WorkshopObjectScript theBed = WorkshopBeds[i] as WorkshopObjectScript if theBed && theBed.GetActorRefOwner() == assignedActor theBed.AssignActor (none) UFO4P_AddUnassignedBedToArray (theBed) endif i += 1 endWhile
should become
while i < countBeds WorkshopObjectScript theBed = WorkshopBeds[i] as WorkshopObjectScript if theBed && theBed.GetActorRefOwner() == assignedActor theBed.AssignActor (none) if !theBed.GetActorRefOwner() UFO4P_AddUnassignedBedToArray (theBed) endif endif i += 1 endWhile
Alternatively, the condition checks could be placed directly in the UFO4P_AddUnassignedBedToArray() and UFO4P_RemoveFromUnassignedBedsArray() functions, which would protect them against incorrect data coming in from other potential calls. A similar modification might be useful for the work objects array helper functions as well. It might also be a good idea to create a UFO4P_RemoveFromActorsWithoutBedsArray() function, analogous to the ones for beds and work objects, to encapsulate logging and basic 'these things exist' checking.
I have a question, and either you or I can put in another ticket if you feel this is related. I run into the same thing a lot of times with vendor stores. I have to send a settler to another settlement then immediately back to clear the store. I have no problems with papyrus and my Active Scripts, Suspended Stacks 1 & 2 are always at about 1, 0, 0. I usually only have about 6 settlers at most settlements and never above 12 at my larger ones.
I'm afraid I've not encountered that one, so I can't speak to whether it's related or not. Sorry.
EDIT:
Although you could try recompiling the WorkshopParentScript in debug mode and see what its workshop logfile has to say?
EDIT:
Although you could try recompiling the WorkshopParentScript in debug mode and see what its workshop logfile has to say?
Comment #4 Sep 17, 2021 12:46 pm
Edited by foamy on Sep 17, 2021 12:47 pm
OK, thank you. It's mainly named NPCs that have the issue. I'll try to track more info down.
Showing Comments 1 - 5 of 5