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

[RFC] Spawn changes #1986

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[RFC] Spawn changes #1986

wants to merge 4 commits into from

Conversation

mugmuggy
Copy link
Contributor

@mugmuggy mugmuggy commented Jul 1, 2021 •
edited

Partially Fixes #649

Describe what the proposed change does

  • Laying the foundation for AI Hospitals, hence the AIHospitals change for hasStaffedDesk
  • Hospital new functions to support reputation based allocation of patients to hospitals
  • World - even though you don't have a staffed desk, TH still counts them towards the statistics, even though no patients are actually spawned.
  • World - assign the disease to the patient, choose the hospital based on the patients disease and spawns the patient to the chosen hospital with the selected disease
  • World - spawn hours align with the 0-49 rather than 1-50 so spawns actually occur the day they should
  • World - onEndMonth removes unnecessary logic, though deprecates getReputationImpact (I have left that in for now maybe I should remove it) - popn config, for the initial start, the first value is used and then at the end of the month the value is used again. L1 6 spawns in Feb.
  • World - update spawn dates removes the math.n_random/math.ceil possibility of returning a number < 1, more closely aligns with TH - mostly fixes First patient takes too long to appear #649
  • World - new functions getNewPatientDisease (refactored from spawnPatient) - chooseHospitalForNewPatient, introduces AllocRand in to the mix
  • Progress report tracks population changes in real time, also allows enables switching between the different hospitals - left unimplemented comment on the tooltip for now, as AIs aren't implemented

Bonus of this one, turn off limit camera and the patients for the AI Hospitals will be strolling around on the edges of the map

@mugmuggy mugmuggy changed the title Spawn changes [RFC] Spawn changes Jul 1, 2021
@lewri
Copy link
Member

lewri commented Jul 1, 2021

Bonus of this one, turn off limit camera and the patients for the AI Hospitals will be strolling around on the edges of the map

I feel sorry for those lost little lambs

(Seriously though, I'll try to look over this tomorrow evening)

TheCycoONE
TheCycoONE reviewed Jul 2, 2021
View reviewed changes
Copy link
Member

@TheCycoONE TheCycoONE left a comment

Choose a reason for hiding this comment

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

Nice PR. Not sure World - even though you don't have a staffed desk, TH still counts them towards the statistics, even though no patients are actually spawned. is actually the right thing to do, but for fidelity I suppose it can stay.

I agree you should remove getRepuationImpact.

CorsixTH/Lua/world.lua
hosp_patient_value[s] = value_proposition
end

local threshold = math.random(0, math.floor(value_proposition - 1))
Copy link
Member

Choose a reason for hiding this comment

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

Nice, though clever enough it deserves a comment (or the comment above expanded) i.e. to include that the hospital is chosen randomly with a chance proportional to it's relative value proposition.

Copy link
Member

Choose a reason for hiding this comment

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

Seconding this... I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the hasStaffedDesk, they don't spawn, but they do count to the visitor count on the annual report but not on the charts/graphs ui, so maybe I should adjust that also. Though I think that will introduce another statistic. Looks like I could repurpose the World.total_population and Hospital.population variables to account for this.

lewri
lewri reviewed Jul 2, 2021
View reviewed changes
Copy link
Member

@lewri lewri left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Hoping the edge spawned patients will be hidden in another patch

CorsixTH/Lua/world.lua Outdated

--! Choose hospital for normal patient
--!param disease (table) - the disease assigned to this patient
--!return hospital (Hospital)
Copy link
Member

Choose a reason for hiding this comment

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

Accidental double space?

CorsixTH/Lua/world.lua
hosp_patient_value[s] = value_proposition
end

local threshold = math.random(0, math.floor(value_proposition - 1))
Copy link
Member

Choose a reason for hiding this comment

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

Seconding this... I think

@mugmuggy mugmuggy changed the title [RFC] Spawn changes [WIP] Spawn changes Jul 3, 2021
@lewri lewri added this to In progress in 0.66 Release via automation Jul 5, 2021
@mugmuggy mugmuggy force-pushed the spawn_changes branch 2 times, most recently from e05868a to 7072085 Compare April 23, 2022 06:25
lewri
lewri reviewed Apr 23, 2022
View reviewed changes
CorsixTH/Lua/hospitals/ai_hospital.lua
@@ -46,6 +46,10 @@ function AIHospital:logTransaction()
-- AI doesn't need a log of transactions, as it is only used for UI purposes
end

function AIHospital:hasStaffedDesk()
Copy link
Member

Choose a reason for hiding this comment

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

Possible future todo?:
Adjust this to only be true when the AI Hospital opens

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed this will switch with the AI hospitals change to actually detect a receptionist has been hired and a desk exists again. This just gets it working before they are implemented

@mugmuggy mugmuggy changed the title [WIP] Spawn changes [RFC] Spawn changes Apr 23, 2022
@lewri lewri moved this from In progress to Missed/Postponed in 0.66 Release May 21, 2022
@lewri lewri added this to In progress in 0.67 Release via automation May 21, 2022
@lewri lewri mentioned this pull request Jul 6, 2022
Merged
@lewri lewri mentioned this pull request Oct 29, 2022
Merged
@lewri lewri moved this from In progress to Missed/Postponed in 0.67 Release Jul 1, 2023
@lewri lewri added this to In progress in 0.68 Release Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheCycoONE TheCycoONE TheCycoONE left review comments

@lewri lewri lewri left review comments

Assignees
No one assigned
Labels
PR:Refactor
Projects
0.68 Release
  
In progress
0.66 Release
  
Missed/Postponed
0.67 Release
  
Missed/Postponed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

First patient takes too long to appear
3 participants
@mugmuggy @lewri @TheCycoONE