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.

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

Refactor earthquakes to its own class #2573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tobylane
Copy link
Member

@tobylane tobylane commented May 22, 2024

Describe what the proposed change does

  • Earthquakes are now a sub class of an instance that belongs to World, with ongoing/next quakes transferred. No additional features. The old recount for current_map_earthquake was faulty so it wasn't copied over (World:afterLoad).

@lewri
Copy link
Member

lewri commented May 22, 2024

I had originally planned myself to make an Event class with Earthquake, VIP, Emergency and Epidemic being subclasses but this very likely works too. Maybe better, because I'm sure all those events above might be too diverse to tie to its own overarching class.

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.

We're definitely moving a lot around, so it's going to be a little challenging to make sure it's all correct.

CorsixTH/Lua/earthquake.lua Show resolved Hide resolved
CorsixTH/Lua/hospitals/player_hospital.lua Outdated Show resolved Hide resolved
ui:playSound("quake.wav")
end
else
print("Unknown stage: " .. stage)
Copy link
Member

Choose a reason for hiding this comment

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

If stage is nil won't this crash?

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be an assert, to always crash on bad input, with a fix for nil stage.

Comment on lines +2159 to +2157
--!param old (integer) The old version of the save game.
--!param new (integer) The current version of the save game format.
Copy link
Member

Choose a reason for hiding this comment

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

The parent of all afterLoads is App. But it does need defining somewhere.

CorsixTH/Lua/world.lua Outdated Show resolved Hide resolved
self.remaining_damage = self.remaining_damage - 1
self.damage_timer = self.damage_timer + damage_time

-- Patient falling: work in progress, see PR 1848
Copy link
Member

Choose a reason for hiding this comment

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

*tips hat*

CorsixTH/Lua/earthquake.lua Outdated Show resolved Hide resolved
CorsixTH/Lua/cheats.lua Outdated Show resolved Hide resolved
@@ -774,6 +774,47 @@ function PlayerHospital:playSound(sound)
end
end

--! The UI parts of earthquake ticks
--!param stage (string) Stage of the active earthquake
function PlayerHospital:tickEarthquake(stage)
Copy link
Member

Choose a reason for hiding this comment

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

The trouble here is an earthquake is a world event. The announcements aren't though.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the UI parts of the world event, though there may be more from competitor hospitals later.

CorsixTH/Lua/hospitals/player_hospital.lua Outdated Show resolved Hide resolved
.luacheckrc Outdated Show resolved Hide resolved
CorsixTH/Lua/world.lua Outdated Show resolved Hide resolved
@lewri lewri added this to In progress in 0.68 Release via automation May 30, 2024
@tobylane
Copy link
Member Author

Made changes from feedback.

---@type Earthquake
local Earthquake = _G["Earthquake"]

-- All fields relating to the current or next earthquake:
Copy link
Member

@lewri lewri Jun 15, 2024

Choose a reason for hiding this comment

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

@tobylane a nit but out of curiosity would a block quote be more sufficient here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
0.68 Release
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants