Conversation
This update creates the RW_WeatherInstance table before defining its functions to prevent nil indexing errors. It also installs hooks for saving and loading additional weather data such as blizzard status and snow forecasts.
KeilerHirsch
left a comment
There was a problem hiding this comment.
The idempotency guard (rw.hooksInstalled) and the nil-checks throughout are genuinely good defensive additions, regardless of the exact original crash — I can't fully reproduce the specific "attempt to index nil" claim from the pre-PR code shown in this diff (RW_WeatherInstance = {} is already on line 1, before any function RW_WeatherInstance:... definition, so the table exists by the time those run in a normal top-to-bottom load) — but re-running the hook-install code on a double source() is a real, known FS25 modding footgun regardless, so the guard is worth having either way.
But I think there's a real, more serious bug introduced here, not fixed: rw.loadFromXMLFile's parameter order is swapped.
function rw.loadFromXMLFile(superFunc, self, xmlFile, key, ...)
...
if superFunc ~= nil then
result = superFunc(self, xmlFile, key, ...)
endThis is registered via Utils.overwrittenFunction(WeatherInstance.loadFromXMLFile, rw.loadFromXMLFile). The calling convention for overwrittenFunction callbacks is (self, superFunc, ...originalArgs) — self first, superFunc second. That's not just my assumption: it's exactly what the old code in this same file used (function RW_WeatherInstance:loadFromXMLFile(superFunc, xmlFile, key), which desugars to (self, superFunc, xmlFile, key) via the : self-binding), and it's the same order I've independently seen in Utils.overwrittenFunction call sites in a couple of other FS25 mods this week (e.g. CabCinematicSpec.onPlayerActionInputEnter(playerInputComponent, superFunc, ...)).
The new plain-function version has no implicit self, so its two named parameters bind positionally to whatever the engine actually passes — meaning the parameter named superFunc receives the real self (the WeatherInstance table), and the parameter named self receives the real superFunc (the original function). Then superFunc(self, xmlFile, key, ...) tries to call the WeatherInstance table as a function, which should raise a Lua error ("attempt to call a table value") the very first time any WeatherInstance loads from a savegame — i.e. on every load, for everyone. That would be worse than the bug this PR sets out to fix.
The fix should just be swapping the two parameter names (self first, superFunc second) to match the established convention — the three appendedFunction hooks (saveToXMLFile/readStream/writeStream) already have this right, it's specifically the one overwrittenFunction hook that's inverted.
Requesting changes on this one specifically because of how severe the failure mode is (every savegame load) — happy to re-review once that's swapped.
Hi, i tried anyway but without success. It's over my knowledge about this complicated mod :-/ |
This update creates the RW_WeatherInstance table before defining its functions to prevent nil indexing errors. It also installs hooks for saving and loading additional weather data such as blizzard status and snow forecasts.