Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CrewContracts - onUpdateBB #5596

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

CrewContracts - onUpdateBB #5596

wants to merge 6 commits into from

Conversation

zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Jul 15, 2023 •
edited
Loading

Introduce onUpdateBB to the CrewContracts module.

Some points:

  • It's unclear to me what happens when a character is deleted when it's busy in an interview (onChat) but I believe another character gets removed (the next in line) instead when the character is employed and taken off the list. In any case the crew ad update doesn't seem to interfere with the onChat action with a candidate and it can be employed and will show up on the payroll.
  • The speed at which the table changes depends on the number of applicants which means most time will be spent in a lower number of crew applicants. You could make the speed depend on the population but I don't thin it disturbs the game in any way. Sometimes there's just low action. You can also set a minimum number of applicants on denser populations.
  • When the number of applicants go to 0 the applicants are reseeded after a while. This has got to be there as the function doesn't recover from 0. The add/remove action is depending on the number of applicants so when the array is empty it needs to be reseeded. Sometimes there are no applicant's and this may be true for quite a while.

Fixes #5510

@impaktor
Copy link
Member

The speed at which the table changes depends on the number of applicants which means most time will be spent in a lower number of crew applicants.

I'm not suggesting any changes to the PR (haven't looked at the code, yet), but if you want to see how number of available ships in the market place was done, I wrote a post on that in #3243, assuming you're facing something similar here, with number of crew contracts, and how these are updated / churn

@zonkmachine
Copy link
Member Author

I wrote a post on that in #3243,

Those plots are mesmerizing...

Gliese852
Gliese852 reviewed Jul 16, 2023
View reviewed changes
data/modules/CrewContracts/CrewContracts.lua Outdated
table.insert(nonPersistentCharactersForCrew[station],newCrew())
print("Reseeding crew candidates")
else
-- 1 in 30 to be removed and then maybe someone new inserted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean 1 in 101? or I do not understand correctly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, old comment, numbers have changed.

Gliese852
Gliese852 reviewed Jul 16, 2023
View reviewed changes
data/modules/CrewContracts/CrewContracts.lua Outdated
if #nonPersistentCharactersForCrew[station] < 1 and Engine.rand:Integer(0, 10) == 0 then -- Not much crew around
table.insert(nonPersistentCharactersForCrew[station],newCrew())
print("Reseeding crew candidates")
else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 times out of 11 this branch will fire, even if the array is empty, although now it will not cause an effect, it looks suspicious.

Copy link
Member Author

@zonkmachine zonkmachine Jul 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are no applicants, array is empty AND a random number equates to 0.
That's one in 11, not 10 times out of 11. The function reseeds the table when it goes to zero and let's it stay at zero for a randomly determined time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking about the "else" branch, yes, I said incorrectly, if the array is empty, the else branch will fire 10 times out of 11.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I go it. Yes, that looks a bit wonky.

Gliese852
Gliese852 reviewed Jul 16, 2023
View reviewed changes
data/modules/CrewContracts/CrewContracts.lua Outdated
end
end
for k,v in pairs(nonPersistentCharactersForCrew[station]) do
if #nonPersistentCharactersForCrew[station] < math.ceil(Game.system.population) * 2 + 1 then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to see an explanation of this criterion, looks like something empirical?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's from this equation a bit higher up on the page.

pioneer/data/modules/CrewContracts/CrewContracts.lua

Lines 394 to 395 in 0f417f2

-- Number is based on population, nicked from Assassinations.lua and tweaked
for i = 1, Engine.rand:Integer(0, math.ceil(Game.system.population) * 2 + 1) do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, maybe then we should put it in a small separate function, with a descriptive name?

@zonkmachine
Copy link
Member Author

zonkmachine commented Jul 16, 2023 •
edited
Loading

@Gliese852 Thanks for the review! I'm currently looking into the algorithm @impaktor linked to above. May need to rewrite this. I like this one though.

@zonkmachine
Copy link
Member Author

zonkmachine commented Jul 17, 2023 •
edited
Loading

Number of applicants over about two and a half months game time. There is no affinity to any mean number but just a free running capitalistic hell. System Administration Resting has max applicants of 11 and Cydonia 39. In these scenarios I have modified the chance of reseeding after hitting zero from one in ten to one in a hundred. That's maybe a bit too long of a wait but I think it should be pretty high. Sometimes things just go bad. :)

graph4

@zonkmachine zonkmachine marked this pull request as draft July 18, 2023 19:21
@zonkmachine
Copy link
Member Author

@impaktor OK, trying your way. I haven't tweaked the values to fit the human market yet.

@zonkmachine
Copy link
Member Author

zonkmachine commented Jul 19, 2023 •
edited
Loading

Graph redone for 2.5 months of human galactic migration. This is with the same variables as for the ship market. Please note that the population base is different between master and this PR. Crew Ad used Game.system.population and the one from the ship market uses station.path:GetSystemBody().parent.population. There is more drama in the earlier code.

graph5

@zonkmachine
Copy link
Member Author

Rebased. When you have the 'Crew for hire" add open, the add isn't refreshed when the available crew changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gliese852 Gliese852 Gliese852 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crew for Hire BBS ad doesn't dynamically spawn
3 participants
@zonkmachine @impaktor @Gliese852