Skip to content

Commit

Permalink
Revert "Return vectors for vehicles component funcs (fixes mantis 9507)"
Browse files Browse the repository at this point in the history 
This reverts commit ee0858e.
  • Loading branch information
@ccw808
ccw808 committed Jun 26, 2017
1 parent 8748d7e commit b47310f
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 62 deletions.
63 changes: 3 additions & 60 deletions Client/mods/deathmatch/logic/luadefs/CLuaVehicleDefs.cpp
View file
Expand Up @@ -189,9 +189,9 @@ void CLuaVehicleDefs::AddClass ( lua_State* luaVM )
lua_classfunction ( luaVM, "getSirenParams", "getVehicleSirenParams" );
lua_classfunction ( luaVM, "getSirens", "getVehicleSirens" );
lua_classfunction ( luaVM, "getSirensOn", "getVehicleSirensOn" );
lua_classfunction ( luaVM, "getComponentPosition", OOP_GetVehicleComponentPosition );
lua_classfunction ( luaVM, "getComponentPosition", "getVehicleComponentPosition" );
lua_classfunction ( luaVM, "getComponentVisible", "getVehicleComponentVisible" );
lua_classfunction ( luaVM, "getComponentRotation", OOP_GetVehicleComponentRotation );
lua_classfunction ( luaVM, "getComponentRotation", "getVehicleComponentRotation" );
lua_classfunction ( luaVM, "getUpgrades", "getVehicleUpgrades" );
lua_classfunction ( luaVM, "getUpgradeSlotName", "getVehicleUpgradeSlotName" );
lua_classfunction ( luaVM, "getCompatibleUpgrades", "getVehicleCompatibleUpgrades" );
Expand Down Expand Up @@ -3040,6 +3040,7 @@ int CLuaVehicleDefs::SetVehicleComponentPosition ( lua_State* luaVM )
return 1;
}


int CLuaVehicleDefs::GetVehicleComponentPosition ( lua_State* luaVM )
{
// float, float, float getVehicleComponentPosition ( vehicle theVehicle, string theComponent [, string base = "root"] )
Expand Down Expand Up @@ -3070,34 +3071,6 @@ int CLuaVehicleDefs::GetVehicleComponentPosition ( lua_State* luaVM )
return 1;
}

int CLuaVehicleDefs::OOP_GetVehicleComponentPosition ( lua_State* luaVM )
{
// float, float, float getVehicleComponentPosition ( vehicle theVehicle, string theComponent [, string base = "root"] )
SString strComponent;
CClientVehicle * pVehicle = NULL;
EComponentBaseType outputBase;

CScriptArgReader argStream ( luaVM );
argStream.ReadUserData ( pVehicle );
argStream.ReadString ( strComponent );
argStream.ReadEnumString ( outputBase, EComponentBase::ROOT );

if ( !argStream.HasErrors () )
{
CVector vecPosition;
if ( pVehicle->GetComponentPosition ( strComponent, vecPosition, outputBase ) )
{
lua_pushvector ( luaVM, vecPosition );
return 1;
}
}
else
m_pScriptDebugging->LogCustom ( luaVM, argStream.GetFullErrorMessage () );

lua_pushboolean ( luaVM, false );
return 1;
}

int CLuaVehicleDefs::SetVehicleComponentRotation ( lua_State* luaVM )
{
// bool setVehicleComponentRotation ( vehicle theVehicle, string theComponent, float rotX, float rotY, float rotZ [, string base = "parent"] )
Expand Down Expand Up @@ -3159,36 +3132,6 @@ int CLuaVehicleDefs::GetVehicleComponentRotation ( lua_State* luaVM )
return 1;
}

int CLuaVehicleDefs::OOP_GetVehicleComponentRotation ( lua_State* luaVM )
{
// float, float, float getVehicleComponentRotation ( vehicle theVehicle, string theComponent [, string base = "parent"] )
SString strComponent;
CClientVehicle * pVehicle = NULL;
EComponentBaseType outputBase;

CScriptArgReader argStream ( luaVM );
argStream.ReadUserData ( pVehicle );
argStream.ReadString ( strComponent );
argStream.ReadEnumString ( outputBase, EComponentBase::PARENT );

if ( !argStream.HasErrors () )
{
CVector vecRotation;
if ( pVehicle->GetComponentRotation ( strComponent, vecRotation, outputBase ) )
{
// Script uses degrees
ConvertRadiansToDegrees ( vecRotation );
lua_pushvector ( luaVM, vecRotation );
return 1;
}
}
else
m_pScriptDebugging->LogCustom ( luaVM, argStream.GetFullErrorMessage () );

lua_pushboolean ( luaVM, false );
return 1;
}

int CLuaVehicleDefs::ResetVehicleComponentPosition ( lua_State* luaVM )
{
CScriptArgReader argStream ( luaVM );
Expand Down
4 changes: 2 additions & 2 deletions Client/mods/deathmatch/logic/luadefs/CLuaVehicleDefs.h
View file
Expand Up @@ -136,9 +136,9 @@ class CLuaVehicleDefs : public CLuaDefs

// Components
LUA_DECLARE ( SetVehicleComponentPosition );
LUA_DECLARE_OOP ( GetVehicleComponentPosition );
LUA_DECLARE ( GetVehicleComponentPosition );
LUA_DECLARE ( SetVehicleComponentRotation );
LUA_DECLARE_OOP ( GetVehicleComponentRotation );
LUA_DECLARE ( GetVehicleComponentRotation );
LUA_DECLARE ( ResetVehicleComponentPosition );
LUA_DECLARE ( ResetVehicleComponentRotation );
LUA_DECLARE ( SetVehicleComponentVisible );
Expand Down

9 comments on commit b47310f

@gatno
Copy link

@gatno gatno commented on b47310f Jun 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should not be so much reckoning on backward compatibility.
OOP getPosition Methods should always return a vector as other getPosition methods.

@nbredikhin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gatno you are actually right, but I don't think that it's a good idea to do compatibility breaking changes in minor releases.

@StiviiK
Copy link

@StiviiK StiviiK commented on b47310f Jun 26, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very in inconsistent compared to other OOP getPosition methods! I totally agree with @gatno!

@PeewX
Copy link

@PeewX PeewX commented on b47310f Jun 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow..... Next step -> Revert "Revert "Return vectors for vehicles component funcs (fixes mantis 9507)""
Thanks. 👍

@AlexTMjugador
Copy link
Member

@AlexTMjugador AlexTMjugador commented on b47310f Jul 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about C++ code, but is it possible to return three doubles when the script expects them (treats return values like numbers, uses the three return values, etc.) and a OOP vector when not? That won't break backward compatibility while being as consistent as possible with other OOP methods.

@gatno , regarding to backward compatibility, I think it can be an issue when scripts are compiled, as modifying them by hand to conform to these changes is difficult.

@qaisjp
Copy link
Contributor

@qaisjp qaisjp commented on b47310f Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexTMjugador

That's not possible without static analysis, unfortunately.

Tbh, imo community scripts shouldn't be compiled, and I doubt any resources on the community make use of this bug (and if they did, there's a high likelihood it is made by an active developer).

@ccw808
Copy link
Member Author

@ccw808 ccw808 commented on b47310f Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our plan is to add a meta.xml <min_mta_version> barrier, where below a certain value the old behaviour is used

@StiviiK
Copy link

@StiviiK StiviiK commented on b47310f Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccw808 this sounds like a good idea!

@AlexTMjugador
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the <min_mta_version> tag seems the perfect balance between complexity, backwards compatibility and OOP functions consistence 👍

Please sign in to comment.