Hi,

thanks, I've added the missing icon.

And yeah the rename icon is not ideal, but anyway all of them can be changed easily. I just picked public domain ones to avoid any licensing issues

-- Auria



On Sat, Mar 22, 2014 at 2:25 PM, Marc Coll <marc.coll.carrillo@gmail.com> wrote:
After pulling the last changes from the main repository I've just realized that there's one icon missing, the one int the main menu. I'm attaching it in this message. I edited it myself from existing STK icons, so no license problems this time.

By the way, I think I like your icons better (except the rename one).

Hi,

thanks for the various clarifications!

I have committed your work to our code repository, thanks a lot for your contribution! (I was thinking you deserve a mention in our credits for your significant contribution. Would you like to be credited by your name, or prefer a nickname?)

On points 5-6, I agree, these can be improved later along other GUI improvements.

For #8, it's very likely that we've never needed before to disable a single button in a toolbar and it's just not implemented properly :( so another thing to do eventually

Regarding icons, thanks. I have changed a few of them, not to look better, but simply for licensing reason; since you did not specify where these icons come from, I replaced them with public domain images just to be sure.

-- Auria



On Thu, Mar 20, 2014 at 2:22 PM, Marc Coll <marc.coll.carrillo@gmail.com> wrote:
OK, let's answer this one by one:

1) I modified the sprite bank because I needed a way to scale images to a predefined size. As far as I know, the setScale function sets a single scale ratio for all the images, which doesn't work well because track screenshots have different sizes. The idea is that my changes only affect you if you use the scaleToHeight() function. If you don't, it should work exactly as it did before. At least in theory.

2) If m_height is 0 (which means that nobody called scaleToHeight), it just does what it did before using the m_scale set by the setScale() function. If there is a height, it returns that height as the width. I know it looks a little bit confusing, and that you would normally expect it to recalculate the width. In fact, that's what it did in the first place, but the list widget didn't like having different width icons and had trouble drawing them, causing the contents of other columns to be misplaced. So I eventually decided to force the icons to be perfectly square, and that seemed to do the trick.

3) You are absolutely right, it should be m_user_config_dir. Apparently, it used to be m_config_dir, but someone changed it while I was working on the file, and I didn't notice it when I did the merging.

4) I thought it would be more consistent to call a single function. But I guess it doesn't really matter, since all three vectors are supposed to have the same number of elements. The compiler would probaly inline the function anyway, but I can change it if you like. No problem.

5) I'm afraid this is a limitation of the list widget, sending an event when just a single click occurred. I think one of the proposals for the GSOC2014 was to improve the GUI system, this could be a good place to start. :)

6) Same as 5).

7) You are right, I've just changed it and it feels a lot better now.

8) Actually, the save button is supposed to deactivate after the changes have been saved, but for some reason I'm not able to do so. I thought calling setDeactivated() would be enough, but it doesn't seem to work. I guess I'm doing something wrong, but I can't figure it out. Do I need to do something else in order to disable an icon button? Supply a disabled version of the icon, perhaps?

9) Sorry about that, I didn't realize. I'm attaching a zip file with all of them.

Thanks for your valuable feedback.


Hi,

Very well done!! The interface works pretty well, and the code seems pretty clean

Just some questions and remarks. Many of these remarks will not prevent including your work, and are only raised as things to do eventually.

1) First, I see you modified the sprite bank used for font rendering. can you explain why? Font rendering is pretty compilcated, we have fallback fonts and all, so there are lots of subtle implications for any change, that you may not notice while testing in english

2) Also, this part doesn't seem quite right :
+s32 STKModifiedSpriteBank::getScaledWidth(s32 width) const
+{
+    if (m_height == 0)
+        return (s32)((float)width * m_scale);
+    else
+        return m_height;
+}


3) File manager no longer builds on Windows, in 'checkAndCreateGPDir', m_config_dir is undeclared. Pretty sure you meant 'm_user_config_dir' ?

4) In many places you do :
-    for(unsigned int i=0; i<m_tracks.size(); i++)
+    for(unsigned int i=0; i<getNumberOfTracks(); i++)

Calling getNumberOfTracks() on every iteration of a foor loop seems like a potential performance issue :
    for(unsigned int i=0; i< getNumberOfTracks(); i++)
The compiler might be able to optimise it away, but I'm not sure what this gives over checking good old m_tracks.size() ? (In theory it would be even better to place the size in a variable like "int size = m_tracks.size" though I have noticed in practice that compilers are usually able to automatically optimize that one)


5) Clicking on an already selected list item edits this item. It can be a little surprising, so ideally we could change that to properly detect a double-click. Of course this rather minor and not a showstopper at all, only a minor note

6) In the track list, you can click on the headers of the list and it highlights them and shows a sort icon, but they aren't actually sortable :) Ideally clicking on list headers should be disabled. Not a showstopper either.

7) The icon bar at the bottom is not fully right, the last item you clicked on remained "selected", but they're actions buttons, you don't select anything. The fix for this simple, just replace <ribbon with <buttonbar

8) It would be nice if when you click on the "save" button there was some feedback that something happened. Clicking on a button and nothing happens is a little unnerving for users :)

9) Finally, note that you patch does not include the png files, so all icons are missing :) Maybe you can send them as a zip?


But very, very nice work. I didn't manage to cause any serious bug in it ;)


On Tue, Mar 18, 2014 at 5:08 PM, Marc Coll <marc.coll.carrillo@gmail.com> wrote:
Hi! After a few days of work I finally have something to show. As
promised, the new grand prix editor is nearly finished, with just some
minor details left to be polished, and I would like to hear what do you
think. Any suggestions are welcome. I have attached a patch with all the
changes.

Please note that the artwork is temporary. I was hoping that some of the
STK artists could help me with it. Meanwhile, the icons are a mixture of
existing STK icons with some minor editing, and some LGPL licensed icons
I found on the Internet. I believe it is OK to include LGPL licensed stuff.

To access the editor simply click on the new icon on the main screen.
I'm not sure if that's the best place to put it, but it seemed logical
to me.

Anyway, please try it and let me know what do you guys think. I'ts my
first contribution, so please don't be too hard. :)

Marc Coll

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Supertuxkart-devel mailing list
Supertuxkart-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/supertuxkart-devel




------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech


_______________________________________________
Supertuxkart-devel mailing list
Supertuxkart-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/supertuxkart-devel


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Supertuxkart-devel mailing list
Supertuxkart-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/supertuxkart-devel




------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech


_______________________________________________
Supertuxkart-devel mailing list
Supertuxkart-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/supertuxkart-devel


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Supertuxkart-devel mailing list
Supertuxkart-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/supertuxkart-devel