View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003123 | FSSCP | scripting | public | 2014-10-06 08:07 | 2014-11-01 11:08 |
| Reporter | m_m | Assigned To | m_m | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.7.1 | ||||
| Target Version | 3.7.2 | ||||
| Summary | 0003123: Virtual variables and indexers that return nil break the scripting system | ||||
| Description | If 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 Reproduce | Have 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. | ||||
| Tags | No tags attached. | ||||
|
|
lua.cpp.patch (541 bytes)
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
}
|
|
|
Is there any way in which this fix can break existing scripts? |
|
|
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. |
|
|
3123.patch (732 bytes)
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
}
|
|
|
May as well just get rid of rval altogether (patch attached as per conversation on #scp), but otherwise, consider this reviewed. |
|
|
Fix committed to trunk@11168. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-10-06 08:07 | m_m | New Issue | |
| 2014-10-06 08:07 | m_m | Status | new => assigned |
| 2014-10-06 08:07 | m_m | Assigned To | => m_m |
| 2014-10-06 08:07 | m_m | File Added: lua.cpp.patch | |
| 2014-10-06 08:08 | m_m | Status | assigned => code review |
| 2014-11-01 00:21 | MageKing17 | Note Added: 0016368 | |
| 2014-11-01 08:17 | m_m | Note Added: 0016369 | |
| 2014-11-01 10:59 | MageKing17 | File Added: 3123.patch | |
| 2014-11-01 11:00 | MageKing17 | Note Added: 0016370 | |
| 2014-11-01 11:08 | m_m | Changeset attached | => fs2open trunk r11168 |
| 2014-11-01 11:08 | m_m | Note Added: 0016371 | |
| 2014-11-01 11:08 | m_m | Status | code review => resolved |
| 2014-11-01 11:08 | m_m | Resolution | open => fixed |
