scene2d, break everyone?

So, scene2d is a bit of a monster API-wise. It makes extensive use of public and protected fields. Converting this to a nice getters and setters would be quite some effort, and would break literally ALL applications that use scene2d. Fixing up your app would be relatively straightforward, and in the end we would have a nicer API. Do you think this is worth everyone’s effort?

-Nate

edit by Mario: i take full responsibilty for this. Scared off by Dalvik’s then abymisal method call overhead i went with public fields which made things quite a bit cumbersome over the last few months/weeks. I’m all for change, benchmarking it should be part of that though. My prediction: the effect won’t be as severe as anticipated. I think now is the time that we get those things fixed, our nightlies, SVN tags and releases should relieve the pain for you guys a little. Mea culpa and feedback more than welcome!

33 thoughts on “scene2d, break everyone?

  1. @aaa, I would say older projects could continue using a specific SVN revision.

    @bach, there is some truth to this, though I try not to worry too much about it as 1) 7x faster than really fast may still be really fast, 2) JIT compilation will inline setters/getters evenutally.

  2. Coming from someone who’s been sinking his teeth into scene2d recently I support the idea of having mutator methods, better APIs always will help attract new users. I found the current use of setting public fields slightly jarring compared to the design philosophy of the rest of the library so anything that helps consistency and writing safer code is fine by me

  3. Breaking changes happen, we’ve got to be agile. Updating code to the new, cleaner API will not that hard, thanks to a good IDE and some basic refactoring skills. You get my vote! 🙂

  4. I also have to add that I hate private members – I just now have to write a completely new TextureAtlas because extending it is not an options (because of private fields that I need to modify – I want to change loading of texture but textures are private). If you could – try to use protected.

  5. Off-topic.

    I’m not sure, that making libgdx a tool “for everything possible use” is good.

    All I needed for my project from libgdx was multiplatform “draw tiles” methods. Unfortunately, you can’t just remove unused modules from it. I think, there should be “lite” ligdx version with just drawing tools, and “full” version with all that additional frameworks and egines.

  6. +1 for clean API – (as I am not using it so far it is straight forward personally) With a powerful IDE like eclipse I cant imagine that following these changes in old projects should be too difficult in order to justify to freeze the API for that reason – also maybe no need to follow if the old projects are finalized anyway ..

  7. No problems here!

    @Piotr: Maybe someone would say “I don’t need the draw tiles method” and another would say “I’m doing a music app so I don’t need the box2d and 3d stuff”, so wouldn’t it be better if all stuff would be in some sort of pluggable modules ?

    When using the source from SVN and compile them yourself (really not that big a deal) and just link the dependencies needed when including them into your game project would be a great solution. However the only thing that should change is the gdx project to be split up into independent modules with a few mandatory base modules. That would cut the size and overview complexity a lot I think.

  8. API breakage is worth it when it’s worth it. I think consistency is extremely important so in this case I’d say fix it.

    Regarding pluggable modules, if you really care about the size of the library it’s quite possible to grab the source and bin lots of the packages you don’t need. I think it’s too easy these days to go “plugin crazy” – last I checked libgdx already had a healthy balance of “core” and “extras” 🙂

  9. What happend with offsetX? In nightly i got errors on this and there was “moveX” field. Is it a definite change?

  10. +1 with aaa : consider leaving as less as possible private member ; prefer protected one.
    +1 / -1 for refactoring: this is not really needed ; in addition to break the compatibility, it will have a big impact on code readability to my mind.

    Also, please consider too the use of composition rather than inheritance everywhere it is possible.
    One concrete example, Actors may better hold a reference to kind if ITouchable interface rather than needing to subclass any Actor subclass (Image, Group, …) just to catch some click events (currently (touchDown/Up/Moved/Dragged functions).

    My 2 cents.

  11. … (about getters/setters): on the other side, about having getter an setter, it would be a nice way to add behaviors when an actor property is modified.

    For example, if I want to make a sprite follow the moves of another sprite, having a setX() function gives a chance to catch any move from the first sprite and react accordingly for the follower one.

    So, it’s finally not a bad idea after all…

    Just for review, if you have some time, you may want to take a look at the (elegant) way sprite properties (position, rotation, color, alpha, scale, …) are implemented into another 2D scenegraph framework dedicated to Java applets: PulpCore

    My … 3 cents 😉

  12. Really, what’s the problem with public fields ? Concerning security is it a problem on architectures like Dalvik (of course not) ? As for the fact it’s error-prone, is it a problem for you (of course it should not) ? Why not just draw a nice chart and decide where should lie what, and then what changes it needs. Ok, blame if it already exists 🙂

  13. I don\’t think why something that works 100% should be changed of aesthetic reasons fully breaking compatibility with all existing software. For all of our projects I would have to recode everything to release a new update, since I extensively use Scene2D everywhere…

  14. I’ve been using libgdx since last year, and I think it’s getting better. The first thing that I noticed before, was that the code was really messy and did not provide the basic setters and getters like most API out there (encap).

    cleaning up the code is a must, and if you are too lazy to update your code or afraid that it might break your application, well then you should probably change your career.

    it’s all part of being a programmer, you always have to keep up with the changes. besides, you can stick with older builds if you don’t want your application to break.

    I vote yes to change

  15. Hi,

    I fully agree to this principle of cleaning things up, despite incompatibilities.

    However, if it is just wrapping every public field in a getter/setter, without reducing the complexity, there is little gain imho.

    (now I’m not so experienced with libgdx, or GUIs in general, so maybe this complexity is actually unavoidable).

    Cheers

  16. I’m all for a simple change such as adding getters and setters, but how could we possibly be talking about this with all the butchering of scene2d?!

    -Storing numbers as strings?! (BaseTableLayout)
    -So many inconsistencies such as setColor and color.set, touchUp returning void
    -No click listeners in anything but tables
    -So many weird quirks where stuff doesn’t scale as well as it used to

  17. @banfur, the strings BaseTableLayout stores are constraints, not necessarily numbers. actor.color.set would change to setColor with the proposed changes to not use public fields. We do have a plan to make it easier to add listeners to actors. Your last item is extremely vague. Things scale better than they used to.

Leave a Reply

Your email address will not be published.