conversion check failed: v.isNumber() (got type undefined) on init

[elexis]

The attached replay (r18136) triggers a warning before the first turn, even before loading the random-map-scripts:

WARNING: JavaScript warning: Script value conversion check failed: v.isNumber() (got type undefined)

Fixing this will likely be trivial. The hard part is figuring out in which file the warning is triggered in the first place. Adding some warn() calls to simulation files should lead there.

[elexis]

Still broken with r18722. Someone should place warn("foo") to some JS files like random map script initialization and some debug_printf calls to the C++ files to figure out where that simulation bug occurs.

[elexis]

Attached replay still bugs on most recent svn (a22 feature freeze time).

1. About the place where the warning occurs:

Added LOGWARNINGs all over the place, still didn't find it.

Decided to search for the warning string and added a breakpoint where it's thrown. Thus got this stack:

Thread #1 [pyrogenesis] 8601 [core: 0] (Suspended : Breakpoint)	
	ScriptInterface::FromJSVal<unsigned int>() at ScriptConversions.cpp:93 0x5c045d	
	ScriptInterface::GetProperty<unsigned int>() at ScriptInterface.h:531 0x4782fe	
	CSimulation2::SetMapSettings() at Simulation2.cpp:797 0x4782fe	
	CMapReader::LoadRMSettings() at MapReader.cpp:1,235 0x6dffc3	
	MemFunThunk<CMapReader>() at LoaderThunks.h:60 0x6dfaa4	
	LDR_ProgressiveLoad() at Loader.cpp:228 0x5f1479	
	LDR_NonprogressiveLoad() at Loader.cpp:318 0x5f1835	
	CReplayPlayer::Replay() at Replay.cpp:179 0x5c94a2	
	RunGameOrAtlas() at main.cpp:519 0x43058e	
	main() at main.cpp:623 0x422747	

So it occurs in L797 of Simulation2.cpp, which is:

	if (!m->m_ComponentManager.GetScriptInterface().GetProperty(m->m_MapSettings, "Seed", seed))
		LOGWARNING("CSimulation2::SetInitAttributes: No seed value specified - using %d", seed);

So we can see a check that doesn't actually work was added and should use HasProperty.

Now the fun thing is that this might occur with many hundreds of GetProperty calls, so someone funny could check all occurances or write unit tests for all occurances.

An alternative would be adding a Has check to Scriptinterface->GetProperty, but that seems wrong because we get the same warning in JS and actually do want to have that warning instead of silently returning undefined, don't we?

If we want to remove that warning altogether, reopen the ticket.

---

2. About the question why that Seed wassn't defined to begin with in gamesetup.js:

It must have been some kind of persist-match-settings bug or map-type changed bug. Before ​Phab:rP18604, the Seed wasn't always set on launchGame but only on loadPersistMatchSettings and selectMapType. Still don't see a way how this could have occured: ​https://code.wildfiregames.com/source/0ad/browse/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js;18127

---

3. Gamesetup.cpp should also always start with Seed, as that is also used to initialize the simulation as of ​Phab:rP18604. But I expect it to not be able to trigger the Simulation2.cpp warning, because CSimulation2::SetMapSettings(JS::HandleValue settings) is only called for random maps, and Gamesetup.cpp always sets it for random maps.