2018-02-19 01:52 EST


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0003123FSSCPscriptingpublic2014-11-01 07:08
Reporterm_m 
Assigned Tom_m 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
Product Version3.7.1 
Target Version3.7.2Fixed in Version 
Summary0003123: Virtual variables and indexers that return nil break the scripting system
DescriptionIf an ADE_VIRTVAR implementation returns ADE_RETURN_NIL the ade_index_handler will not return that value but will try to call the indexer of the object. If there is none or if it also returns nil the scripting system will raise an error saying that the index could not be found in the referenced type.
To fix this the index handler should always return the value of a called function even if it is nil, the attached patch makes the necessary changes.
Steps To ReproduceHave a ADE_VIRTVAR implementation return ADE_RETURN_NIL. As soon as that VirtVar is accessed a LuaError will be shown claiming that the specified index could not be found.
TagsNo tags attached.
Attached Files
  • patch file icon lua.cpp.patch (541 bytes) 2014-10-06 04:07 -
    Index: code/parse/lua.cpp
    ===================================================================
    --- code/parse/lua.cpp	(revision 11102)
    +++ code/parse/lua.cpp	(working copy)
    @@ -16413,8 +16413,7 @@
     				//WMC - Return as appropriate
     				int rval = lua_gettop(L) - vvt_ldx;
     
    -				if(rval)
    -					return rval;
    +				return rval;
     			}
     			else
     			{
    @@ -16446,8 +16445,7 @@
     
     			int rval = lua_gettop(L) - err_ldx;
     
    -			if(rval)
    -				return rval;
    +			return rval;
     		}
     		lua_pop(L, 2);	//WMC - Don't need __indexer or error handler
     	}
    
    patch file icon lua.cpp.patch (541 bytes) 2014-10-06 04:07 +
  • patch file icon 3123.patch (732 bytes) 2014-11-01 06:59 -
    Index: code/parse/lua.cpp
    ===================================================================
    --- code/parse/lua.cpp	(revision 11167)
    +++ code/parse/lua.cpp	(working copy)
    @@ -16427,11 +16427,7 @@
     				//Execute function
     				lua_pcall(L, numargs, LUA_MULTRET, err_ldx);
     
    -				//WMC - Return as appropriate
    -				int rval = lua_gettop(L) - vvt_ldx;
    -
    -				if(rval)
    -					return rval;
    +				return (lua_gettop(L) - vvt_ldx);
     			}
     			else
     			{
    @@ -16461,10 +16457,7 @@
     			//Execute function
     			lua_pcall(L, last_arg_ldx, LUA_MULTRET, err_ldx);
     
    -			int rval = lua_gettop(L) - err_ldx;
    -
    -			if(rval)
    -				return rval;
    +			return (lua_gettop(L) - err_ldx);
     		}
     		lua_pop(L, 2);	//WMC - Don't need __indexer or error handler
     	}
    
    patch file icon 3123.patch (732 bytes) 2014-11-01 06:59 +

-Relationships
+Relationships

-Notes

~0016368

MageKing17 (developer)

Is there any way in which this fix can break existing scripts?

~0016369

m_m (developer)

There is a very specific case where a VirtVar returns nil and the indexer of the object actually returns the value. I can't think of a script working correctly if it uses this bug.
I don't think this could break existing scripts.

~0016370

MageKing17 (developer)

May as well just get rid of rval altogether (patch attached as per conversation on #scp), but otherwise, consider this reviewed.

~0016371

m_m (developer)

Fix committed to trunk@11168.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2014-10-06 04:07 m_m New Issue
2014-10-06 04:07 m_m Status new => assigned
2014-10-06 04:07 m_m Assigned To => m_m
2014-10-06 04:07 m_m File Added: lua.cpp.patch
2014-10-06 04:08 m_m Status assigned => code review
2014-10-31 20:21 MageKing17 Note Added: 0016368
2014-11-01 04:17 m_m Note Added: 0016369
2014-11-01 06:59 MageKing17 File Added: 3123.patch
2014-11-01 07:00 MageKing17 Note Added: 0016370
2014-11-01 07:08 m_m Changeset attached => fs2open trunk r11168
2014-11-01 07:08 m_m Note Added: 0016371
2014-11-01 07:08 m_m Status code review => resolved
2014-11-01 07:08 m_m Resolution open => fixed
+Issue History