Review Request: Achievements 3: AchievementsAsset

[Laszlo Papp]

> On July 12, 2011, 1:32 p.m., Arjen Hiemstra wrote:
> > engine/gluon_engine_export.h, lines 94-101
> > <http://git.reviewboard.kde.org/r/101909/diff/2/?file=26705#file26705line94>
> >
> >     While it is currently done this way in many assets/components it is not necessary to export them. As far as I know, the only reason we originally started exporting them was due to KDE's settings setting visibility("hidden") by default. This means we can avoid exporting them altogether which should not be done anyway. I have not seen any indication that this causes problems on windows either.
> 
> Felix Rohrbach wrote:
>     Laszlo seems to have some doubts about this working on windows, but he can't test it right now. Is it ok for you if I undo this change for this patch now and you / someone else can do this change on master for all assets later?
> 
> Laszlo Papp wrote:
>     It probably works on windows, but I had so much trouble with windows previously so that I wanted to just make a quick test for it. 
>     
>     I think without this removal the code would remain more consistent in the current state without causing an issue since right now all the assets and components are exported. Hence the situation would be that this one is not exported, and if I do not know this background information, I am just wondering why not and probably take some time with it unneccesarily.
>     
>     We can also remove them later in one step. I am also fine with a relevant commit message, if it is an issue to keep the consistency.

We have just had a conversation with Andrius (Xiluembo) and we cannnot remove them. Static libraries would be fine, but Windows DLLs need to have the export/import.

Asset example:
git blame -L 30,30 ../engine/assets/audio/sound/soundasset.h
237b5f74 (Andrius da Costa Ribas 2011-01-25 17:09:08 -0200 30)     class GLUON_ASSET_SOUND_EXPORT SoundAsset : public Asset

Component example:
git blame -L 33,33 ../engine/components/graphics/cameracontroller/cameracontrollercomponent.h
237b5f74 (Andrius da Costa Ribas 2011-01-25 17:09:08 -0200 33)     class GLUON_COMPONENT_CAMERACONTROLLER_EXPORT CameraControllerComponent : public Component

Hence they were added on Windows by purpose half a year ago when we first tried to properly make Gluon on Windows. I am fine with the patch if this removal is reverted for now.


- Laszlo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101909/#review4645
-----------------------------------------------------------


On July 12, 2011, 2:30 p.m., Felix Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101909/
> -----------------------------------------------------------
> 
> (Updated July 12, 2011, 2:30 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Adds the AchievementsAsset class, which cares about the Achievement objects and saves them in a file. Also add mime-type information for this file type.
> 
> Note: This patch also adds the file "achievements_template.gluonachievements" at engine/assets/other/achievements/. As this file is empty, ReviewBoard doesn't show it.
> 
> 
> Diffs
> -----
> 
>   creator/x-gluon-mimetypes.xml 378876d 
>   engine/assets/CMakeLists.txt b8c087b 
>   engine/assets/other/achievements/CMakeLists.txt PRE-CREATION 
>   engine/assets/other/achievements/achievementsasset.h PRE-CREATION 
>   engine/assets/other/achievements/achievementsasset.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101909/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/gluon/attachments/20110717/13863898/attachment-0001.htm