HomeWildfire Games

Check for and save mod version for savegames and replays
AuditedrP21239

Description

Check for and save mod version for savegames and replays

Reviewed by: elexis
Fixes: #4887
Differential Revision: https://code.wildfiregames.com/D955

Event Timeline

elexis added a subscriber: elexis.Feb 20 2018, 1:18 AM
elexis added inline comments.
/ps/trunk/source/ps/Mod.cpp
116

That would probably be better in the header file as people looking for things they can use externally look in that file first.
If they don't read the code, they might assume they get all mods.

Angen added a subscriber: Angen.EditedFeb 21 2018, 8:37 PM

using debug mode, crash at MOZ_ASSERT(mStatementDone);, after starting game

       pyrogenesis_dbg.exe!mozilla::detail::GuardObjectNotificationReceiver::~GuardObjectNotificationReceiver() Line 100	C++
	pyrogenesis_dbg.exe!JS::Rooted<JS::Value>::~Rooted<JS::Value>() Line 783	C++
	pyrogenesis_dbg.exe!CReplayLogger::StartGame(JS::MutableHandle<JS::Value> attribs) Line 65	C++
	pyrogenesis_dbg.exe!CGame::StartGame(JS::MutableHandle<JS::Value> attribs, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & savedState) Line 369	C++
	pyrogenesis_dbg.exe!JSI_Game::StartGame(ScriptInterface::CxPrivate * pCxPrivate, JS::Handle<JS::Value> attribs, int playerID) Line 52	C++
	pyrogenesis_dbg.exe!ScriptInterface_NativeWrapper<void>::call<void __cdecl(ScriptInterface::CxPrivate *,JS::Handle<JS::Value>,int),JS::Handle<JS::Value>,int>(JSContext * cx, JS::MutableHandle<JS::Value> __formal, void (ScriptInterface::CxPrivate *, JS::Handle<JS::Value>, int) * fptr, JS::Handle<JS::Value> <params_0>, int <params_1>) Line 86	C++
	pyrogenesis_dbg.exe!ScriptInterface::call<void,JS::Handle<JS::Value>,int,&JSI_Game::StartGame>(JSContext * cx, unsigned int argc, JS::Value * vp) Line 125	C++
	mozjs38-ps-debug-vc120.dll!js::CallJSNative(JSContext * cx, bool (JSContext *, unsigned int, JS::Value *) * native, const JS::CallArgs & args) Line 226	C++
	mozjs38-ps-debug-vc120.dll!js::Invoke(JSContext * cx, JS::CallArgs args, js::MaybeConstruct construct) Line 498	C++
	mozjs38-ps-debug-vc120.dll!Interpret(JSContext * cx, js::RunState & state) Line 2602	C++
	mozjs38-ps-debug-vc120.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 448	C++
	mozjs38-ps-debug-vc120.dll!js::Invoke(JSContext * cx, JS::CallArgs args, js::MaybeConstruct construct) Line 517	C++
	mozjs38-ps-debug-vc120.dll!js::Invoke(JSContext * cx, const JS::Value & thisv, const JS::Value & fval, unsigned int argc, const JS::Value * argv, JS::MutableHandle<JS::Value> rval) Line 554	C++
	mozjs38-ps-debug-vc120.dll!JS_CallFunctionValue(JSContext * cx, JS::Handle<JSObject *> obj, JS::Handle<JS::Value> fval, const JS::HandleValueArray & args, JS::MutableHandle<JS::Value> rval) Line 4216	C++
	pyrogenesis_dbg.exe!IGUIObject::ScriptEvent(const CStr8 & Action) Line 472	C++
	pyrogenesis_dbg.exe!IGUIObject::SendEvent(EGUIMessageType type, const CStr8 & EventName) Line 448	C++
	pyrogenesis_dbg.exe!IGUIButtonBehavior::HandleMessage(SGUIMessage & Message) Line 77	C++
	pyrogenesis_dbg.exe!CButton::HandleMessage(SGUIMessage & Message) Line 83	C++
	pyrogenesis_dbg.exe!IGUIObject::SendEvent(EGUIMessageType type, const CStr8 & EventName) Line 444	C++
	pyrogenesis_dbg.exe!CGUI::HandleEvent(const SDL_Event_ * ev) Line 154	C++
	pyrogenesis_dbg.exe!CGUIManager::HandleEvent(const SDL_Event_ * ev) Line 352	C++
	pyrogenesis_dbg.exe!gui_handler(const SDL_Event_ * ev) Line 49	C++
	pyrogenesis_dbg.exe!in_dispatch_event(const SDL_Event_ * ev) Line 62	C++
	pyrogenesis_dbg.exe!PumpEvents() Line 186	C++
	pyrogenesis_dbg.exe!Frame() Line 335	C++
	pyrogenesis_dbg.exe!RunGameOrAtlas(int argc, const char * * argv) Line 590	C++
	pyrogenesis_dbg.exe!SDL_main(int argc, char * * argv) Line 632	C++
	pyrogenesis_dbg.exe!main_utf8(int argc, char * * argv) Line 126	C
	pyrogenesis_dbg.exe!wmain(int argc, unsigned short * * wargv, unsigned short * wenvp) Line 151	C
elexis added inline comments.Feb 21 2018, 8:39 PM
/ps/trunk/source/ps/Mod.cpp
110

This patch was the first that converted the array to an object first and then called SetPropertyInt, would not be surprised if that would result in something such as that error.

can reproduce

/ps/trunk/source/ps/Mod.cpp
110

then we need to return to scriptInterface.Eval("([])", &ret);

elexis raised a concern with this commit.May 23 2018, 12:12 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/gui/loadgame/load.js
118

This now consumes 170ms per selection on my machine with only zip (public).
Now imagine there are 10 mods and it consumes 1.7 seconds each time the player selects a different savegame.

/ps/trunk/binaries/data/mods/public/gui/replaymenu/replay_menu.js
4

This global is unneeded once the call is only 20 microseconds.

This commit now has outstanding concerns.May 23 2018, 12:12 PM
Stan added a subscriber: Stan.May 23 2018, 12:33 PM
Stan added inline comments.
/ps/trunk/source/ps/Mod.cpp
116

Does that mean you can have special changes in there like I don't know a language pack without affecting other users ?

wraitii added inline comments.
/ps/trunk/source/ps/Mod.cpp
116

I think these should be considered "core" mods that pyrogenesis requires and mounted separately from game mods (such as those that 0 A.D. requires)

elexis added inline comments.May 24 2018, 12:50 PM
/ps/trunk/source/ps/Mod.cpp
116

The condition can likely be superseded by finding out if it changes the simulation state or not: #5053.

All concerns with this commit have now been addressed.May 24 2018, 8:31 PM