dedicated server broken due to calls to g_gr

Bug #1322869 reported by Nasenbaer on 2014-05-24
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Undecided
Unassigned

Bug Description

user fu-fu wrote in our forums, that the dedicated server module is segfaulting once the game is started and indeed it is.
The problem (until now - didn't came further) is the preloading of the animations of bobs and immovables. During loading the bobs and immovables, the animations are now supposed to be loaded as well. However this should only be the case if Widelands is run with user interface, as else this does not make a lot of sense obviously.

Now I am a bit unsure how to treat this problem. Attached you find the first part of a fix I was working on. For me it looks a lot like a hack. Generally it would be better to not at all call the animation loader, however in that case profile.cc is bailing out as some sections have not be used...

Well I hoped to get a fast fix ready, but I stopped at this point as I was
* unsure as described above
* not seeing an end until now (everytime I fixed one place where g_gr is called, the next appeared), so I thought once more whether there wasn't a better way to fix this?
* ... I still am rare on time. :(

Can anyone of the graphic code guys please take a look at this?

Related branches

Nasenbaer (nasenbaer) wrote :
SirVer (sirver) wrote :

I think it makes more sense to implement a dummy g_gr that has all methods, but they do not do anything. Having ifs() all over the codebase is not ideal.

That entails: make all methods in Graphic virtual and add virtual dummys for most other classes in there that will returned instead.

The proper fix is of course to have the logical being fully observable and never calling the GUI, then you would just never launch the GUI - but that is a lot of work.

Nasenbaer (nasenbaer) wrote :

I agree with you that the dummy g_gr would mostlikely be the easiest solution. However that dummy would still be forced to e.g. readout certain data, as Widelands does check_used() checks on those data, which is just wasted CPU and time... :(

Concerning "the proper fix" - it was that way in Build 17, so unfortunally this is more or less a regression :-/.

SirVer (sirver) wrote :

> readout certain data, as Widelands does check_used() checks on those data, which is just wasted CPU and time... :(

not sure what you want to say with that? checking that values of configuration files are used is certainly a good thing to do - it will prevent future bugs. If you do not care for the dummy implementation, add a 'mark all keys as read' call to Section and LuaTable.

> Concerning "the proper fix" - it was that way in Build 17, so unfortunally this is more or less a regression :-/.

No, we never had logic and UI separated. Try building logic/* economy/* into one library. If this works without needed headers or libraries from ui_basic or wui/ or graphics/ we would be there, but that never works and will not work for a long time.

concerning the regression: untested code regresses easily, especially one that relies on if/else checks in many places of the code. Add a regression test to the testsuite and it will not regress again.

SirVer (sirver) wrote :

Setting to incomplete for bug sweeping.

Changed in widelands:
status: Confirmed → Incomplete
GunChleoc (gunchleoc) on 2015-11-07
Changed in widelands:
status: Incomplete → Confirmed
SirVer (sirver) wrote :

Then we should have a test for dedicated as well somewhere.

SirVer (sirver) on 2016-02-06
Changed in widelands:
assignee: nobody → SirVer (sirver)
assignee: SirVer (sirver) → nobody
assignee: nobody → SirVer (sirver)
SirVer (sirver) wrote :

--dedicated is gone, so this bug is gone too.

Changed in widelands:
status: Confirmed → Fix Committed
assignee: SirVer (sirver) → nobody
GunChleoc (gunchleoc) on 2016-10-10
tags: added: graphics
removed: graphic
GunChleoc (gunchleoc) on 2016-10-25
Changed in widelands:
status: Fix Committed → Fix Released
GunChleoc (gunchleoc) wrote :

Fixed in build19-rc1.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Related blueprints