Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005844opensim[REGION] Scripting Enginepublic2012-01-08 15:042019-11-06 07:44
Reporterjcochran 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusnewResolutionopen 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version 
Summary0005844: NULL key value in in statement does not evaluate as false.
DescriptionThe NULL key "00000000-0000-0000-0000-000000000000" should evaluate as FALSE inside a conditional. See http://wiki.secondlife.com/wiki/NULL_KEY [^]

Steps To ReproduceDrop the following script into any prim and let it run.

===== CUT HERE ======
default
{
    state_entry()
    {
        key id = (key) NULL_KEY;
        
        if (id) {
            llSay(0, "Claims to be 'TRUE'. Value = " + (string) id);
        } else {
            llSay(0, "Claims to be 'FALSE'. Value = " + (string) id);
        }
    }
}
TagsNo tags attached.
Git Revision or version number32eb7dd-r/17733
Run Mode Standalone (Multiple Regions)
Physics EngineBasicPhysics
Script Engine
EnvironmentMono / Windows
Mono Version2.10
Viewer
Attached Filespatch file icon 0001-Evaluate-if-key_var-as-false-when-the-key-key_var-ha.patch [^] (974 bytes) 2018-08-20 08:01 [Show Content]

- Relationships

-  Notes
(0020651)
jcochran (reporter)
2012-01-09 13:46

Looking at the code, I've found so far the following snippet

================== CUT HERE ===============
namespace OpenSim.Region.ScriptEngine.Shared.CodeTools
{
    public class LSL2CSCodeTransformer
    {
        private SYMBOL m_astRoot = null;
        private static Dictionary<string, string> m_datatypeLSL2OpenSim = null;

        /// <summary>
        /// Pass the new CodeTranformer an abstract syntax tree.
        /// </summary>
        /// <param name="astRoot">The root node of the AST.</param>
        public LSL2CSCodeTransformer(SYMBOL astRoot)
        {
            m_astRoot = astRoot;

            // let's populate the dictionary
            if (null == m_datatypeLSL2OpenSim)
            {
                m_datatypeLSL2OpenSim = new Dictionary<string, string>();
                m_datatypeLSL2OpenSim.Add("integer", "LSL_Types.LSLInteger");
                m_datatypeLSL2OpenSim.Add("float", "LSL_Types.LSLFloat");
                //m_datatypeLSL2OpenSim.Add("key", "LSL_Types.key"); // key doesn't seem to be used
                m_datatypeLSL2OpenSim.Add("key", "LSL_Types.LSLString");
                m_datatypeLSL2OpenSim.Add("string", "LSL_Types.LSLString");
                m_datatypeLSL2OpenSim.Add("vector", "LSL_Types.Vector3");
                m_datatypeLSL2OpenSim.Add("rotation", "LSL_Types.Quaternion");
                m_datatypeLSL2OpenSim.Add("list", "LSL_Types.list");
            }
        }
================== CUT HERE ===============

Un-commenting the line:

   //m_datatypeLSL2OpenSim.Add("key", "LSL_Types.key"); // key doesn't seem to be used

and commenting the line:

   m_datatypeLSL2OpenSim.Add("key", "LSL_Types.LSLString");

Causes the expected behavior for some trivial test scripts. However, for some non-trivial scripts, am getting parse errors. And the non-trivial scripts compile correctly with type "key" being considered as a synonym for "string" as is currently implemented.
(0032866)
kcozens (administrator)
2018-08-20 08:03
edited on: 2018-08-20 08:05

I have attached a simple patch that fixes this problem. I don't expect it will have any adverse side effects but I suggest you try it for yourself.

NOTE: The patch can be applied to both the 0.8.2 and 0.9 code versions.

(0032867)
BillBlight (developer)
2018-08-20 13:45

This has been this way forever the way it is normally used is ,

if(key == NULL_KEY)

or similar !=NULL_KEY

where this patch could cause issue is if you are checking for the presence of a key at all, but not sure ...
  ..
(0032868)
BillBlight (developer)
2018-08-20 14:05

I'm wondering if this could break , osIsUUID(String thing) because it seems to me this would always substitute a valid key format even if it is NULL_KEY ?
(0032869)
kcozens (administrator)
2018-08-21 10:03

This problem has existed for a long time as can be determined by the age of this bug report. I had someone ask me about this recently as they ran were bitten by the problem.

The patch won't change the behaviour of osIsUUID() as that is a function call.
(0032871)
FreakyTech (reporter)
2018-08-23 23:40

That patch will actually break string to boolean compares.
string test=NULL_KEY;
if(test) llOwnerSay("string is non-empty");

The problem is that string to boolean has a different behavior than key to boolean.
To fix that kind of issue you have to separate the types.
(0035834)
melanie (administrator)
2019-11-06 02:08

Since YEngine fixes this problem, I don't think that potential XEngine script breakage is worth it.
(0035835)
tampa (reporter)
2019-11-06 03:13

Unless there is a concrete plan to completely drop XEngine in the next release fixing things should still be on the agenda, after all we still fix bugs in Bullet too.
(0035836)
Haravikk (reporter)
2019-11-06 07:34

Just wanted to note since this was being discussed, but it's not just NULL_KEY that is supposed to be considered invalid for key to boolean, but all invalid keys should return false, i.e- any that aren't exactly 36 characters of hexadecimal with four dashes in the correct places. One of the horrible sad realities of Linden Labs deciding that keys should be multi-byte strings in a memory constrained environment *shudders*.

For example, in LSL on SL you can do the following:

    key invalidKey = "foo bar";
    if (invalidKey) { llOwnerSay("Key is valid"); }
    else { llOwnerSay("Key is invalid"); }

i.e- storing invalid data in a key is fine, but it will fail a conditional check, which is how you can test for it when reading keys from a notecard, user input etc.

But if keys are handled purely as strings this isn't possible, as there's no such thing as an invalid string, meaning the test should be completely different for keys.
(0035837)
UbitUmarov (administrator)
2019-11-06 07:44

Yes it should, and is on Y now
but due the the key duality, that was never acheived on X, for a reason or another.
and try to fix one case, breaks others.

so as before, do explicit compares with NULL_KEY, etc
check for valid uuid usind osIsUUID(..)
use () to enforce your order of evaluation of things..
etc.

- Issue History
Date Modified Username Field Change
2012-01-08 15:04 jcochran New Issue
2012-01-09 13:46 jcochran Note Added: 0020651
2018-08-20 08:01 kcozens File Added: 0001-Evaluate-if-key_var-as-false-when-the-key-key_var-ha.patch
2018-08-20 08:03 kcozens Note Added: 0032866
2018-08-20 08:05 kcozens Note Edited: 0032866 View Revisions
2018-08-20 13:45 BillBlight Note Added: 0032867
2018-08-20 14:05 BillBlight Note Added: 0032868
2018-08-21 10:03 kcozens Note Added: 0032869
2018-08-23 23:40 FreakyTech Note Added: 0032871
2019-11-06 02:08 melanie Note Added: 0035834
2019-11-06 03:13 tampa Note Added: 0035835
2019-11-06 07:34 Haravikk Note Added: 0035836
2019-11-06 07:44 UbitUmarov Note Added: 0035837


Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker