duplication/disappearance crew on list - after dismiss

[radius75]

I repeatedly hired and fired the employee Released crewman sometimes disappears or... replicates itself

[radius75]

Until the game crashes(freeze), after click "Make offer..."

[radius75]

Released crewman

not the only one, sometimes the other person disappears from the list after 'dismiss'

[impaktor]

Well, for one, the facedata still lingers if you click "go back" instead of "hang up". I can try to see if I can fix at least that. Maybe I also can see what's causing this duplication.

[radius75]

Mostly I click F3 instead "go back"/ "hang up" ;)

[impaktor]

I just managed to get a crash. That's a good thing, Three persons were on offer. I hired two, and the third went missing. Fired the two, and they show up in the Crews for Hire. Third person still missing.

Error: [string "[T] @modules/CrewContracts/CrewContracts.lua"]:269: bad argument #1 to 'remove' (position out of bounds)

Error: [string "[T] @modules/CrewContracts/CrewContracts.lua"]:269: bad argument #1 to 'remove' (position out of bounds)
stack traceback:
    [C]: in function 'remove'
    [string "[T] @modules/CrewContracts/CrewContracts.lua"]:269: in function <[string "[T] @modules/CrewContracts/CrewContracts.lua"]:154>
    (...tail calls...)
    [string "[T] @ui/StationView/ChatForm.lua"]:71: in function <[string "[T] @ui/StationView/ChatForm.lua"]:67>

EDIT: I suspect "Ask candidate to sit test" is doing something bad

[walterar]

It is related to #2631

[impaktor]

Are you sure? I think the CrewContracts misplaces one character, and then when it tries to find it again it crashes.

[walterar]

It's another symptom of the same problem, broken registry.

[impaktor]

I've been trying to come up with a reliable way to reproduce the problem, (using only Go back, Hang up, and Ask candidate to sit a test), result is inconclusive. Hiring a bunch and then sacking them doesn't put them back on the list.

Have not been able to see multiple characters as in the screenshot through.

[radius75]

using only Go back, Hang up, and Ask candidate

try F3:

Mostly I click F3 instead "go back"/ "hang up" ;)

[robn]

If forcibly quitting out of the form via F3 is what's triggering it, then the problem may be that the module is not updating its state immediately on change. I had the same bug with with RemoveAdvertOnClose - setting up an action to happen when the form closed was no use if you couldn't control all the ways the form could be closed.

[radius75]

proposing a solution: deactivate all HUD buttons and KEY F1,F2...(Esc... everything:), when opening the window with 'missions', 'hire crew', etc.. is it possible?

[radius75]

examples of exceptions: printscreen :)

[impaktor]

I'd just like to add that I got the disappearing crew members, and the crash,sometimes after just a few clicks, without ever using anything but the mouse.

So all for all my comments above I was playing without using the keyboard.

[demolitions]

Fiddling with the CrewContracts module (in looking into https://github.com/pioneerspacesim/pioneer/issues/3456), I found out that the module has some issues with adding/removing crewmembers into its local registry, I had experienced this issue too, will look into it.

[demolitions]

What is the expected behaviour? When the crewman is hired, he should disappear from the available list (does not currently). When he is dismissed, should he return immediately on the list?

[Brianetta]

I wouldn't expect a crew member to immediately present him- or herself for hire after being dismissed. If they did, I'd expect them to be extraordinarily difficult to re-hire. In the event that they are willing to be re-hired, they should demand a pay rise.

When I first added the CrewContracts I wanted the crew to be difficult, unpredictable and human. If you messed about with them during negotiations, I wanted it to be ridiculously unlikely that they'd accept any offer from you at all, since you're not showing them any respect. I wanted it to be extremely difficult to work the system, by punishing the player for trying to game the potential crew they were interviewing.

The same should go for having them re-appear once sacked. Unless their contract is up naturally, they should be upset at having lost their job, and suspicious when the player attempts to re-hire them immediately. Some of them will, like ordinary humans, take the opportunity for a few fays unpaid leave, or even to find a new line of work - so some should vanish permanently.

Of course, hired crewmembers should no longer appear in the "available for hire" list.

[demolitions]

@Brianetta what you're saying explains to me the reasons of specific choices I found in the code, once dismissed the candidate returns immediately to the list, but with increased wage request, and decreased playerRelationship, which sometimes result in "your offer isn't attractive to me", but nonetheless he is added back to the list of potential candidates.

I have been able to reproduce the duplicate candidates when dismissed, and the disappearing candidate when I hired another, and I think I have fixed the issue at hand (simple matter of botched table indexes), tomorrow I will conduct some more tests and forge a pull request if everything seems ok.

[demolitions]

Should I open a feature request to change the behaviour by "delaying" the return of the candidate to the list? The current code (with or without my fix) adds the candidate back as soon as he gets dismissed.

[Brianetta]

If the candidate has neither been hired or dismissed, but has simply been interviewed by the player, they should remain on the list in the bulletin board. The bulletin board is aimed at any and all pilots visiting the station, not just the player, so should not be changed simply because the player spoke to one of them without hiring. No crew member waiting for a job will leave the list just because one ship's captain wasn't good enough.

Of course, that individual's opinion of the player might have changed, and is stored in case the player talks to them again. The player absolutely doesn't get a fresh start, although negotiations might superficially appear that way to begin with.

[demolitions]

@Brianetta you're right, I was talking only about the "after being dismissed" event, not about the pre-hiring negotiation, but I failed to explain myself.

The negotiation before hiring does not remove or change any list at all, it changes only the "estimatedWage" property of the single candidate, as far as I can tell.

The candidate gets removed from the list only if he accepts the offer and transfers to the player ship.

[Brianetta]

Well, that all sounds good. Of course, it's been a couple of years since I touched this code, so do feel free to take ownership of it . You don't need my permission to make the game better, but you have my good will anyway. (-:

[impaktor]

If the crew shows up on the "for hire" list after being dismissed it gives the player some feeling of consistency and continuation, like "hey, I fired that guy, good luck finding a job", or "damn, I don't want to hire you again", but in sure, in real life it would be some time lag before you put up an advert. It could be some random waiting time (from 0 to t) before the crew member shows up again. I'm fine either way.

@demolitions as you might have noticed, crew members don't do much at the moment, other than letting the player take off with a big ship which requires a crew. If you feel like working more on this, there's plenty of space for you. I'm not sure what ideas were for crew when added (@Brianetta?) but navigator could shorten hyperdrive warm up and jump time, depending on navigation skill, and, I don't know. Brainstorm.

I'd like to see the player assign permanent tasks/stations for the crew. Like when you have one member he's "navigation, turrets, engineering...", and when you have more, they can each have their separate station, allowing them to refine their skills. I guess this discussion should go on the dev. forum. I suspect there might already be some post there, just can't remember right now.

[impaktor]

I'm not sure what ideas were for crew when added

Answer: http://pioneerwiki.com/wiki/Crew

[demolitions]

Will brainstorm on that, I'll try to find the post in the dev forum, I could create a pull request for this fix now, I haven't been able to reproduce the issue all this morning, since it changes the same file as #3465, would it be better to wait for the other to be merged before adding this?

[impaktor]

since it changes the same file would it be better to wait for the other to be merged before adding this?

Well, it's merged now, but for the future: If it changes the same lines, but are separate fixes, then you could either open one pull request, with two commits, each fixing one issue or do one PR, wait for merge and then the next. I'd personaly go with first option, simpler and faster IMO. If you know the changes are on different lines in the same file, then there shouldn't be any conflict, and you could have two PRs from the same master branch.

[demolitions]

Got it, so this time instead I'll pull the merged code from upstream and create another pull request with this fix.