I have very little time this weekend, but wanted to give you quick feedback
right away.
[moving of dirs and splitting graphic into sub directories]
great change.
> So, I decided to do some overhaul:
Would have been better to do that in trunk, so that the data move is
independent. Do you think that is feasible still or whould that be a ton of
work?
Also, I am a bit confused about the direction you took with your overhaul.
Could you give some comments on the design choices you made?
I think addressing files by their name relative to the data directory (which
used to be the root directory of widelands, but now will be data) is fine. It
is similar to how we include headers, always from the root.
1. Introducing sub directories for sound and so on seems cleaner, but might
limit us in the future. For example what about sound effects specific to one
building - should that not live in the same directory where the building is
defined? Think about the ability to define a new building in a scenario for
example, there everything will be in the same directory for sure. I argue this
should be the same for bundled data.
2. We are still not free to rename stuff because of Lua/Scenarios. And the
fiddling with the filenames example could have been avoided without the
image_catalogue too - for example in the one fix you posted, you could have
introduce a string get_fsel_pic_for(int player) which would have killed the
repetition.
I see some disadvantages for the image_catalogue:
- It is similar to the image_cache and it becomes confusing when to use which.
- The code has now one more indirection whenever you need a picture and is
harder to read ("pics/blub.png" is easier than kBlubImageFileName).
- The loading adds more dependence on the g_gr singleton whenever you want an
image.
- Also, what about the enums? Are they send over the network or saved? If so,
we must never change their ordering and cannot ever delete an entry. That is
also not very flexible.
On the other side I see no real advantage to it, since we still cannot move
images around. How do you think this improved the code?
3. we'll figure it out :)
4. It was to be expected that the data move would be a huge change, so it is
perfectly fine to get that right first and then do a separate change for the
shaders.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1397500/+merge/243860
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1397500 into lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
