ufoaiorg / ufoai

UFO:Alien Invasion
http://ufoai.org
GNU General Public License v2.0
148 stars 51 forks source link

ui lua integration: full feature implementation #20

Closed rxadmin closed 9 years ago

rxadmin commented 9 years ago

Renewed pull request, created a clean(er) history using rebase. This feature adds lua integration to the user interface. The implementation works side-by-side the existing .ufo scripts.

How do I merge it with the main branch?

DarkRainX commented 9 years ago

You mean merge with the official repository? If so I think an admin should give the OK first, unfortunately we haven't seen a lot of them lately :(

rxadmin commented 9 years ago

Ah, I see. I'll try to PM mattn.

mgerhardy commented 9 years ago

We still need to rebase properly to remove the merge commits and stuff - but after this is done, I think it's up to DarkRainX to decide this - as he is currently the maintainer (I'm more or less inactive atm).

DarkRainX commented 9 years ago

Rebasing should be trivial, I already did it once before to check the previous PR

@rxadmin if this gets merged when would be a good time? — as in when will yo be around to help? (You know in case something doesn't work completely as expected)

rxadmin commented 9 years ago

Now would be a good time. I'll be around until last week of july. Otherwise starting from halfway august. In between I'm somewhere on a sunny (I hope) beach.

As for the rebasing: I had some problems with it mostly caused by me being inexperienced with the feauture. I finally managed it after a few tries.

DarkRainX commented 9 years ago

Very well then, I've pulled your latest master to a branch in my local repo and rebased that on top of the latest "official" master, merge commits are gone, with minimal noise from the rebase, now it seems that _assets_base.ufo is not up to date with the latest changes, shouldn't be that hard for me to bring it up to date, but just in case, I remember having read something about documentation in the commit logs, how do I get it?

rxadmin commented 9 years ago

I've already added documentation to the wiki. One page explains the working of lua scripts, the seconds gives a reference of the API. Search for "LUA". It should give enough info to update _assets_base.ufo. Should you get stuck, just put a message here or PM me.

DarkRainX commented 9 years ago

Ok, I updated _assests_base.ufo and compiled, there seems to be a problem in the files using ufoscrip when overriding nodes from components created with lua, this breaks all base view windows except the main one (haven't checked others), example as found in hospital.ufo, aircraft.ufo and others:

window <window_name>
{
    {
        fill        true
    }
    // BaseContainerPanel component created with lua
    BaseContainerPanel base_container {
        // This node won't show in the hospital window: seems to have size default to (0,0)
        // instead of inheriting it from the component (other windows specify a size for this node)
        // with { size "<X> <Y>" } here
        panel mainBody {
            // This node won't show in any of the base sections!
            // Again seems to default to a size of (0, 0)
            panel header {
                {}
                string title
                {
                    // No idea if string is set here, most likely not (see below),
                    // but can't see the header at all anyway
                    string          "_Hospital"
                }
            }

        // <Base section specific nodes here>

        // This string is never set in any of the base sections
        data wnd_name { string "<base section name here>" }
    }

Any idea why could this be? It works fine if the component is created with ufoscript

rxadmin commented 9 years ago

I'll look into it. For the lua I did rewrite the cloning/duplicating of nodes but it used to work. Can you push the modificiations on _assets_base.ufo to the fork repo so I can pick them up?

DarkRainX commented 9 years ago

Sure thing I'll push them

rxadmin commented 9 years ago

I've found the problem. It is not in the code, it is in the lua-version of _assets_base.ufo. The node "BaseContainerPanel_mainBody" was created with parent "BaseContainerPanel_credits", a string node were it should be parented to "BaseContainerPanel", the panel node. I've commited a fix to the repo.

rxadmin commented 9 years ago

In the process of debugging, also stumbled on another bug in the call mechanism.

DarkRainX commented 9 years ago

Ah, yes that explains it, much better now, will test a bit more, just to be sure things are ok.

Also our test suit (testall) will need to be fixed to work with this PR: a lot of the test cases are calling UI_Init() without calling CL_Init() first, which leads to a segfault when trying to register the ui nodes, if I add a call to CL_InitLua before the call to UI_Init it will suffice, right?

rxadmin commented 9 years ago

Yes. CL_InitLua() needs to come first. CL_ShutdownLua() should be called to clean up.

BTW: this init stuff is pretty complex... time to clean up??

DarkRainX commented 9 years ago

Ah yes I must not forget to shut it down

You are right, but I doubt that we have the man power right now...

DarkRainX commented 9 years ago

Oh maybe worth to mention sourceforge (which host our main repository) has been experiencing problems and currently some services are down — including repositories, please stand by for merge

rxadmin commented 9 years ago

Already noticed it. However I was unaware the main repository was located there. Thought we had our own server.

BTW: Did you noticed the toolchain needs to be expanded with SWIG to process the .i interface definition? It is not needed for the actual merge, however it is needed for future development. I guess it also should be included in the UfoAI_Win32_build_environment_x.x.x.exe.

DarkRainX commented 9 years ago

No, we are using SF, there's been talk about moving the repo somewhere else or hosting it ourselves before, but nothing ever came of it

Yes I've seen SWIG mentioned before, I guess this is a good time ask: What should know about SWIG? I've never used it before, seems to be handy, might even be worth converting the lua ai to make use of it some day.

rxadmin commented 9 years ago

In short: SWIG translates an interface description for a script language (e.g. lua) to a working binding. The advantage is that the interface description becomes portable and that you can migrate to newer versions of a script language of any other language more easily. I'll add a page to the Wiki explaining more about SWIG.

DarkRainX commented 9 years ago

Ok, so how do I produce ui_lua_shared.cpp form ui_lua_shared.i? is 'swig -c++ -lua' enough?

BTW the repo at SF is back online, I could try the merge now, when are you leaving for the beach? (If you haven't gone already that is)

rxadmin commented 9 years ago

Almost. The command I use (from within C::B) is: swig -c++ -lua $includes -o $file_dir/$file_name.cpp $inputfile Where $inputfile is the .i file, -o specifies the output c of cpp file. Note that if you specify cpp here the compiler must be set to compile this cpp file as ordinary c file to avoid name mangling (wich will result in linker errors).

Below is a line from the build log on my machine to clarify it.

---------------------------------------------- line from the build log

swig -c++ -lua -I....\src\libs\mxml\ -I....\src\libs\mumble\ -I....\src\libs\lua -IZ:_Development\Tools\MinGW\include -IZ:_Development\Tools\MinGW\include\SDL -o Z:_Development\Projects\Ufo\ufoai_rxadmin\src\client\ui\swig/ui_lua_shared.cpp

Z:_Development\Projects\Ufo\ufoai_rxadmin\src\client\ui\swig\ui_lua_shared.i

Als for the beach: I'll leave on tuesday evening.

2015-07-26 0:58 GMT+02:00 DarkRainX notifications@github.com:

Ok, so how do I produce ui_lua_shared.cpp form ui_lua_shared.i? is 'swig -c++ -lua' enough?

BTW the repo at SF is back online, I could try the merge now, when are you leaving for the beach? (If you haven't gone already that is)

— Reply to this email directly or view it on GitHub https://github.com/ufoai/ufoai/pull/20#issuecomment-124909856.

DarkRainX commented 9 years ago

Ah I see, will have to note it down somewhere

Tuesday? Well, that doesn't leave a lot of time to deal with potential issues and while I'm fairly sure that nothing will blow up in your absence, I'd prefer to wait... I guess in the mean time I will prepare things in a new branch at the main repo

rxadmin commented 9 years ago

Agree. Tuesday doesn't leave much time. Wise to wait. Guess we didn't counted on the sourceforge problems. I'll be back somewhere on the 12th. I'll be curious to see if in the meantime you come across any other issues.

2015-07-27 5:47 GMT+02:00 DarkRainX notifications@github.com:

Ah I see, will have to note it down somewhere

Tuesday? Well, that doesn't leave a lot of time to deal with potential issues and while I'm fairly sure that nothing will blow up in your absence, I'd prefer to wait... I guess in the mean time I will prepare things in a new branch at the main repo

— Reply to this email directly or view it on GitHub https://github.com/ufoai/ufoai/pull/20#issuecomment-125076538.

DarkRainX commented 9 years ago

All right then, have a good travel and holiday

DarkRainX commented 9 years ago

Just documenting what I've found:

Another case of wrongly parented nodes in _assets.ufo — fixed on my end

The add_mail confunc in mailclient.ufo seems to be receiving malformed parameters, I'm investigating the issue Edit: Fixed see a059678966f0c941a043fb8f23a14e9fe653bc9d

DarkRainX commented 9 years ago

preventtypingescape needs to be implemented for missionbriefing window otherwise exiting it with escape prevents the player's soldiers from spawning...

I might be able to put a workaround in place for now.

rxadmin commented 9 years ago

I'm back. Just reviewing the status. Can you push modifications to the rxadmin branch? Specifically the _assets.ufo and the ui_lua.cpp fixes (-> malformed parameters issue).

As for the preventtypingescape: it needs to be fixed in the SWIG interface file. I'll push a fix to this branch.

DarkRainX commented 9 years ago

Welcome back! Hope you had a good holiday :)

Sure thing, You want only those two commits? If so let me figure how to PR just those ones, or if you are ok with getting my whole branch just tell me and I'll PR that instead.

rxadmin commented 9 years ago

Had a great holiday. Lots of sun and sea.

The whole branch will do if it's easier for you. After all, the end goal is to merge :-)

DarkRainX commented 9 years ago

Well, it seems to be ready, just needs uncommenting the preventtypypingescape from missionbriefing.ufo (which I probably should have done in my branch and added to my PR while I was there...) and should be good to go — unless you have some reason to hold on the merge?

rxadmin commented 9 years ago

As far as I can see, it's a go for the merge. I'll update the wiki, including the do...end construct. Best have everyone follow the same code style from day one.

DarkRainX commented 9 years ago

Well then, merging now!

Just note that since Mattn asked to remove the merge commits I'll rebase this on top of master before merging, this might cause merge conflicts next time you pull from the master branch

rxadmin commented 9 years ago

Ok. I've linked the wiki pages about the use of lua for the ui scripting. As for merge conflicts. I probably will grab a new copy of the master branch.

DarkRainX commented 9 years ago

Great! Thanks for everything!

Also you can close this PR now if you want — I did mention before that I don't have the rights IIRC