philmasterplus
Active member
Currently (r20617), there are three ways of retrieving the numeric ID of an enumerated data type (e.g. Item, Effect, Familiar):
It also allows types to be implicitly compared by ID:
While this may be convenient, it may cause surprising behaviors, now and in the future. Thus, I propose removing the custom
Why remove a good feature?
Because we are sitting on a bomb.
Consider this expression:
What should the item be converted to, a number or a string?
According to the ECMAScript 6 Language Specification†, the runtime must first convert both operands to a primitive value using a
But it just "works" in KoLmafia! What is going on?
This is caused by a bug in Rhino. To observe this bug, evalute this code in r20617:
Since this behavior is not standards-compliant, it cannot be reproduced in other JavaScript runtimes. This undermines development and testing scripts in environments such as Node.js.
Meanwhile, a contributor has already implemented a fix. Should this make it into the next release of Rhino and we choose to integrate it, it would break all JS code that expects item-with-string concatenation to "just work".
This alone is a strong reason to ditch
† I also checked the ECMAScript 5.1 spec, which is worded differently, but the important parts are the same.
- Using the
toInt()
library function - Using the
id
property (only available for a few types) - Using the custom
valueOf()
method
JavaScript:
let item = Item.get('hair spray');
Number(item); // 744
+item; // 744
String(item); // "hair spray"
print(item); // Prints "hair spray"
It also allows types to be implicitly compared by ID:
JavaScript:
Item.get('cold wad') > Item.get('hot wad'); // true
Item.get('cold wad') < Item.get('hot wad'); // false
While this may be convenient, it may cause surprising behaviors, now and in the future. Thus, I propose removing the custom
valueOf()
implementation altogether.Why remove a good feature?
Because we are sitting on a bomb.
Consider this expression:
JavaScript:
Item.get('pail') + someVar;
What should the item be converted to, a number or a string?
According to the ECMAScript 6 Language Specification†, the runtime must first convert both operands to a primitive value using a
ToPrimitive()
operation. How ToPrimitive()
works deserves its own article, but the gist is:- Call
obj.valueOf()
. If it returns a primitive value, use it. - Otherwise, call
obj.toString()
. If it returns a primitive value, use it. - Throw a
TypeError
.
item.valueOf()
first. Since it returns a primitive value (the number 294), it does not proceed to step 2. Therfore, our pail must always evaluate to the number 294, regardless of the value of someVar
. It should never evaluate to the string "pail"
.But it just "works" in KoLmafia! What is going on?
This is caused by a bug in Rhino. To observe this bug, evalute this code in r20617:
JavaScript:
Item.get('pail') + "foo"; // "pailfoo" (technically a bug, even though it's what we want)
Item.get('pail') + String("foo"); // "294foo" (correct)
let someVar = "foo";
Item.get('pail') + someVar; // "294foo" (correct)
Since this behavior is not standards-compliant, it cannot be reproduced in other JavaScript runtimes. This undermines development and testing scripts in environments such as Node.js.
Meanwhile, a contributor has already implemented a fix. Should this make it into the next release of Rhino and we choose to integrate it, it would break all JS code that expects item-with-string concatenation to "just work".
This alone is a strong reason to ditch
valueOf()
as it stands today. If we delete it, the ToPrimitive()
operation will always convert enumerated types to strings, thereby eliminating the potential for writing incorrect code that relies on accidentally-correct-but-not-guaranteed behavior.† I also checked the ECMAScript 5.1 spec, which is worded differently, but the important parts are the same.
Last edited: