2024-03-28 12:54 CET

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0004361Spring engineLuapublic2014-04-13 15:59
Reportersilentwings 
Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionno change required 
Product Version96.0 
Target VersionFixed in Version 
Summary0004361: gadget:GameSetup second argument argument returned does not work correctly
DescriptionThe second argument returned from (unsynced) gadget:GameSetup is a bool that (according to wiki) controls whether or not the default list of playernames is shown, coloured to indicate each players ready state.

In fact, it only does so for player who are already ready - if a player is not ready then the list of playernames is shown regardless of what is returned in GameSetup.
Steps To Reproducetest using:

function gadget:GameSetup(state,ready,playerStates)
    Spring.Echo(ready)
    if ready then
        return true, false
    else
        return false, false
    end
end

Start a game where you have to choose your owns tartpoints. The list of playernames on the left side of the screen only disappears after you click ready.
TagsNo tags attached.
Checked infolog.txt for Errors
Attached Files
  • patch file icon patch.patch (1,661 bytes) 2014-04-13 14:46 -
    From 643521b13935ad3c673e1890b79ac44ded5915b2 Mon Sep 17 00:00:00 2001
    From: silentwings <moo@moo.moo>
    Date: Sun, 13 Apr 2014 13:42:11 +0100
    Subject: [PATCH] Add third argument to gadget:GameSetup, return true/false to
     hide/show the ready button & player list
    
    ---
     rts/Lua/LuaHandle.cpp | 30 ++++++++++++++++++------------
     1 file changed, 18 insertions(+), 12 deletions(-)
    
    diff --git a/rts/Lua/LuaHandle.cpp b/rts/Lua/LuaHandle.cpp
    index a979186..651d015 100644
    --- a/rts/Lua/LuaHandle.cpp
    +++ b/rts/Lua/LuaHandle.cpp
    @@ -2254,7 +2254,7 @@ bool CLuaHandle::GameSetup(const string& state, bool& ready,
     		return false;
     	}
     	LUA_CALL_IN_CHECK(L, false);
    -	luaL_checkstack(L, 5, __FUNCTION__);
    +	luaL_checkstack(L, 6, __FUNCTION__);
     	static const LuaHashString cmdStr("GameSetup");
     	if (!cmdStr.GetGlobalFunc(L)) {
     		return false;
    @@ -2270,21 +2270,27 @@ bool CLuaHandle::GameSetup(const string& state, bool& ready,
     		lua_pushsstring(L, it->second);
     		lua_rawseti(L, -2, it->first);
     	}
    -
    +
     	// call the routine
    -	if (!RunCallIn(L, cmdStr, 3, 2))
    -		return false;
    -
    -	if (lua_isboolean(L, -2)) {
    -		if (lua_toboolean(L, -2)) {
    -			if (lua_isboolean(L, -1)) {
    -				ready = lua_toboolean(L, -1);
    +	if (!RunCallIn(L, cmdStr, 3, 3))
    +		return false;
    +
    +	if (lua_isboolean(L, 1)) {
    +		if (lua_toboolean(L, 1)) {
    +			if (lua_isboolean(L, 2)) {
    +				ready = lua_toboolean(L, 2);
     			}
    -			lua_pop(L, 2);
    -			return true;
     		}
     	}
    -	lua_pop(L, 2);
    +
    +    if (lua_isboolean(L, 3)) {
    +        if (lua_toboolean(L, 3)) {
    +            lua_pop(L, 3);
    +            return true;
    +        }
    +    }
    +
    +	lua_pop(L, 3);
     	return false;
     }
     
    -- 
    1.8.4.msysgit.0
    
    
    patch file icon patch.patch (1,661 bytes) 2014-04-13 14:46 +

-Relationships
+Relationships

-Notes

~0013001

cleanrock (reporter)

I might be off here but i think you are looking at http://springrts.com/wiki/Lua:Callins which i think is incorrect, http://springrts.com/wiki/LuaCallinReturn shows what matches the code:
  GameSetup() --> string "state", boolean "ready", table "playerStates"
  return success, newReady

~0013002

silentwings (reporter)

http://springrts.com/wiki/LuaCallinReturn is (now orphaned and ) replaced by http://springrts.com/wiki/Lua:Callins. But yes that's what I was reading.

Judging by LuaHandle.cpp you are right - it says "if success then ready = newReady"

BUT it's still true (and unexplained) that the example I gave above does hide the list of playernames only after "return true,false"; maybe it's the return value from CLuaHandle::GameSetup which is controlling this?

~0013003

FLOZi (reporter)

Fixed Lua:Callins page

~0013004

silentwings (reporter)

Last edited: 2014-04-13 13:25

View 4 revisions

Note that the success variable is actually superfluous - the callin is passed the players current ready state and a gadget could just choose return that as newReady.

I would still be very grateful for the ability to hide that list of names (there is enoguh info sent to lua now to fully replace it and I have a complicated to explain reason for wanting to - "ready" in BA now means something slightly different to "green" on that list)

I can submit a patch (I think) that just adds a third argument to be returned, with the third argument being a bool that hides the list of names. If I did this and it worked
(1) would it get used?
(2) what format do you want patches in?

(I have set up code::blocks in Windows for Spring and, with some custom build flags for Cmake that behe gave me, it works beautifully!)

~0013005

Kloot (developer)

Last edited: 2014-04-13 14:26

View 2 revisions

"I would still be very grateful for the ability to hide that list of names"

as you already found out, "it's the return value from CLuaHandle::GameSetup which is controlling this" (aka the FIRST value returned from gadget:GameSetup, called "success" on the wiki)

~0013006

silentwings (reporter)

Last edited: 2014-04-13 14:51

View 2 revisions

No - the 'success' variable only controls that if the player is set to ready. See https://github.com/spring/spring/blob/develop/rts/Lua/LuaHandle.cpp#L2250 - it can't return true unless the player is ready.

I've attached a patch that adds a third value to control it, afaik without breaking existing code.
Base gadgethandler would need updating to match (I don't have basecontent in my local repo) with

function gadgetHandler:GameSetup(state, ready, playerStates)
  local success, newReady, hideList, hList
  for _,g in ipairs(self.GameSetupList) do
    success, newReady, hList = g:GameSetup(state,ready,playerStates)
    hideList = hideList or hList
  end
  return success, newReady, hideList
end

~0013007

Kloot (developer)

Last edited: 2014-04-13 15:16

View 3 revisions

"No - the 'success' variable only controls that if the player is set to ready"

incorrect

CLuaHandle::GameSetup
  // -2 is the index of the FIRST return value
  if (lua_isboolean(L, -2)) {
    if (lua_toboolean(L, -2)) {
      ...
      return true;

GameSetupDrawer::Draw
  if (eventHandler.GameSetup(startState, playerHasReadied, playerStates)) {
    ...
    return;

~0013008

silentwings (reporter)

Last edited: 2014-04-13 15:53

View 3 revisions

Oh, you are quite right! Sorry.

But, without returning success==true and consequently hiding the player list, I can't change the ready state of the player. (Although I didn't need to do that anyway - since this has now become quite a convoluted ticket I suggest to close it.)

+Notes

-Issue History
Date Modified Username Field Change
2014-04-12 20:26 silentwings New Issue
2014-04-13 08:31 cleanrock Note Added: 0013001
2014-04-13 10:17 silentwings Note Added: 0013002
2014-04-13 10:17 FLOZi Note Added: 0013003
2014-04-13 10:26 silentwings Note Added: 0013004
2014-04-13 10:28 silentwings Note Edited: 0013004 View Revisions
2014-04-13 10:31 silentwings Note Edited: 0013004 View Revisions
2014-04-13 13:25 silentwings Note Edited: 0013004 View Revisions
2014-04-13 14:22 Kloot Note Added: 0013005
2014-04-13 14:26 Kloot Note Edited: 0013005 View Revisions
2014-04-13 14:46 silentwings Note Added: 0013006
2014-04-13 14:46 silentwings File Added: patch.patch
2014-04-13 14:51 silentwings Note Edited: 0013006 View Revisions
2014-04-13 15:14 Kloot Note Added: 0013007
2014-04-13 15:16 Kloot Note Edited: 0013007 View Revisions
2014-04-13 15:16 Kloot Note Edited: 0013007 View Revisions
2014-04-13 15:33 silentwings Note Added: 0013008
2014-04-13 15:35 silentwings Note Edited: 0013008 View Revisions
2014-04-13 15:53 silentwings Note Edited: 0013008 View Revisions
2014-04-13 15:59 cleanrock Status new => closed
2014-04-13 15:59 cleanrock Resolution open => no change required
+Issue History