Fix use of 'undefined' sound group on death.

Warn about implicit script/native type conversions.

This was SVN commit r7527.

binaries/data/mods/public/gui/session_new/selection.js

@@ -17,8 +17,10 @@ function _setMotionOverlay(ents, enabled)
 function EntitySelection()
 {
 	// Private properties:
-	this.selected = {}; // { id: 1, id: 1, ... } for each selected entity ID 'id'
-	this.highlighted = {}; // { id: 1, ... } for mouseover-highlighted entity IDs
+	this.selected = {}; // { id:id, id:id, ... } for each selected entity ID 'id'
+	this.highlighted = {}; // { id:id, ... } for mouseover-highlighted entity IDs
+		// (in these, the key is a string and the value is an int; we want to use the
+		// int form wherever possible since it's more efficient to send to the simulation code)
 	this.motionDebugOverlay = false;
 
 	// Public properties:
@@ -37,7 +39,7 @@ EntitySelection.prototype.toggle = function(ent)
 	{
 		_setHighlight([ent], g_ActiveSelectionColour);
 		_setMotionOverlay([ent], this.motionDebugOverlay);
-		this.selected[ent] = 1;
+		this.selected[ent] = ent;
 	}
 	this.dirty = true;
 };
@@ -50,7 +52,7 @@ EntitySelection.prototype.addList = function(ents)
 		if (!this.selected[ent])
 		{
 			added.push(ent);
-			this.selected[ent] = 1;
+			this.selected[ent] = ent;
 		}
 	}
 	_setHighlight(added, g_ActiveSelectionColour);
@@ -69,8 +71,8 @@ EntitySelection.prototype.reset = function()
 EntitySelection.prototype.toList = function()
 {
 	var ents = [];
-	for (var ent in this.selected)
-		ents.push(+ent); // convert from string to number and push
+	for each (var ent in this.selected)
+		ents.push(ent);
 	return ents;
 };
 
@@ -80,7 +82,7 @@ EntitySelection.prototype.setHighlightList = function(ents)
 	var added = [];
 
 	// Remove highlighting for the old units (excluding ones that are actively selected too)
-	for (var ent in this.highlighted)
+	for each (var ent in this.highlighted)
 		if (!this.selected[ent])
 			removed.push(ent);
 	
@@ -98,7 +100,7 @@ EntitySelection.prototype.setHighlightList = function(ents)
 	// Store the new list
 	this.highlighted = {};
 	for each (var ent in ents)
-		this.highlighted[ent] = 1;
+		this.highlighted[ent] = ent;
 };
 
 EntitySelection.prototype.SetMotionDebugOverlay = function(enabled)

binaries/data/mods/public/simulation/components/Health.js

@@ -116,7 +116,7 @@ Health.prototype.CreateCorpse = function()
 
 	// Make it fall over
 	var cmpCorpseVisual = Engine.QueryInterface(corpse, IID_Visual);
-	cmpCorpseVisual.SelectAnimation("death", true);
+	cmpCorpseVisual.SelectAnimation("death", true, 1.0, "");
 };
 
 

binaries/data/mods/public/simulation/components/UnitAI.js

@@ -311,7 +311,7 @@ UnitAI.prototype.SelectAnimation = function(name, once, speed, sound)
 	if (!cmpVisual)
 		return;
 
-	var soundgroup = "";
+	var soundgroup;
 	if (sound)
 	{
 		var cmpSound = Engine.QueryInterface(this.entity, IID_Sound);
@@ -319,6 +319,14 @@ UnitAI.prototype.SelectAnimation = function(name, once, speed, sound)
 			soundgroup = cmpSound.GetSoundGroup(sound);
 	}
 
+	// Set default values if unspecified
+	if (typeof once == "undefined")
+		once = false;
+	if (typeof speed == "undefined")
+		speed = 1.0;
+	if (typeof soundgroup == "undefined")
+		soundgroup = "";
+
 	cmpVisual.SelectAnimation(name, once, speed, soundgroup);
 };
 

source/ps/CLogger.cpp

@@ -145,6 +145,8 @@ void CLogger::WriteError(const wchar_t* message)
 void CLogger::WriteWarning(const wchar_t* message)
 {
 	++m_NumberOfWarnings;
+	if (m_UseDebugPrintf)
+		debug_printf(L"WARNING: %ls\n", message);
 
 	if (g_Console) g_Console->InsertMessage(L"WARNING: %ls", message);
 	*m_InterestingLog << L"<p class=\"warning\">WARNING: "<< message << L"</p>\n";

source/scriptinterface/ScriptConversions.cpp

@@ -20,14 +20,19 @@
 #include "ScriptInterface.h"
 
 #include "ps/utf16string.h"
+#include "ps/CLogger.h"
 
 #include "js/jsapi.h"
 
-#define FAIL(msg) do { JS_ReportError(cx, msg); return false; } while (false)
+#define FAIL(msg) STMT(JS_ReportError(cx, msg); return false)
+
+// Implicit type conversions often hide bugs, so warn about them
+#define WARN_IF_NOT(c) STMT(if (!(c)) { JS_ReportWarning(cx, "Script value conversion check failed: %s", #c); })
 
 template<> bool ScriptInterface::FromJSVal<bool>(JSContext* cx, jsval v, bool& out)
 {
 	JSBool ret;
+	WARN_IF_NOT(JSVAL_IS_BOOLEAN(v));
 	if (!JS_ValueToBoolean(cx, v, &ret))
 		return false;
 	out = (ret ? true : false);
@@ -37,6 +42,7 @@ template<> bool ScriptInterface::FromJSVal<bool>(JSContext* cx, jsval v, bool& o
 template<> bool ScriptInterface::FromJSVal<float>(JSContext* cx, jsval v, float& out)
 {
 	jsdouble ret;
+	WARN_IF_NOT(JSVAL_IS_NUMBER(v));
 	if (!JS_ValueToNumber(cx, v, &ret))
 		return false;
 	out = ret;
@@ -46,6 +52,7 @@ template<> bool ScriptInterface::FromJSVal<float>(JSContext* cx, jsval v, float&
 template<> bool ScriptInterface::FromJSVal<i32>(JSContext* cx, jsval v, i32& out)
 {
 	int32 ret;
+	WARN_IF_NOT(JSVAL_IS_INT(v));
 	if (!JS_ValueToECMAInt32(cx, v, &ret))
 		return false;
 	out = ret;
@@ -55,6 +62,7 @@ template<> bool ScriptInterface::FromJSVal<i32>(JSContext* cx, jsval v, i32& out
 template<> bool ScriptInterface::FromJSVal<u32>(JSContext* cx, jsval v, u32& out)
 {
 	uint32 ret;
+	WARN_IF_NOT(JSVAL_IS_INT(v));
 	if (!JS_ValueToECMAUint32(cx, v, &ret))
 		return false;
 	out = ret;
@@ -76,6 +84,7 @@ template<> bool ScriptInterface::FromJSVal<CScriptValRooted>(JSContext* cx, jsva
 
 template<> bool ScriptInterface::FromJSVal<std::wstring>(JSContext* cx, jsval v, std::wstring& out)
 {
+	WARN_IF_NOT(JSVAL_IS_STRING(v));
 	JSString* ret = JS_ValueToString(cx, v);
 	if (!ret)
 		FAIL("Argument must be convertible to a string");
@@ -86,6 +95,7 @@ template<> bool ScriptInterface::FromJSVal<std::wstring>(JSContext* cx, jsval v,
 
 template<> bool ScriptInterface::FromJSVal<std::string>(JSContext* cx, jsval v, std::string& out)
 {
+	WARN_IF_NOT(JSVAL_IS_STRING(v));
 	JSString* ret = JS_ValueToString(cx, v);
 	if (!ret)
 		FAIL("Argument must be convertible to a string");

source/simulation2/components/CCmpMinimap.cpp

@@ -139,6 +139,10 @@ public:
 
 			const CMessageOwnershipChanged& msgData = static_cast<const CMessageOwnershipChanged&> (msg);
 
+			// If there's no new owner (e.g. the unit is dying) then don't try updating the colour
+			if (msgData.to == -1)
+				break;
+
 			// Find the new player's colour
 			CmpPtr<ICmpPlayerManager> cmpPlayerManager(context, SYSTEM_ENTITY);
 			if (cmpPlayerManager.null())