← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/one_tribe into lp:widelands

 

Thanks for the review!

Regarding the lookup tables
http://bazaar.launchpad.net/~widelands-dev/widelands/one_tribe/revision/7464

We do need the compatibility, because map loading has a building packet etc. I only included those entities in it that can't be avoided for map loading. I decided to rename world entities as well, because now all names correspond to their descnames - I expect that this will make the code easier to read for modders and for new programmers coming in.

Regarding lua_map
http://bazaar.launchpad.net/~widelands-dev/widelands/one_tribe/revision/7465

I need the building directory info to find the helptexts() file. I can't keep the helptexts in C++ because this would make the localization markup too inflexible - it would block the use of ngettext and splitting up of long strings. Alternatively, I could store the whole helptexts script path in C++ instead.

Regarding animations
http://bazaar.launchpad.net/~widelands-dev/widelands/one_tribe/revision/7466#src/logic/instances.cc

MapObjectDescr::add_directional_animation is still needed. The Lua stuff just creates a Lua table that then fits this function.

Regarding sets
http://bazaar.launchpad.net/~widelands-dev/widelands/one_tribe/revision/7466#src/logic/tribes/tribe_descr.h
I am using a set in TribeDescr because a vector would have holes in it, and it would then be expensive to iterate over only the items that aren't nullptr and to calculate the size - the indices are used as WareIndex, so access is better with a set.


[Num Glob vs list_directory]
Sounds like a plan - I will add this to my "todo" bug. I would like to get this branch in this weekend, because I will need time to fix up the translations. So, I don't want to change the code in this branch too much now.

I will look into the rest of the NOCOMS and see what I can fix up until then.


-- 
https://code.launchpad.net/~widelands-dev/widelands/one_tribe/+merge/274832
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/one_tribe.


References