News:

The Forum Rules and Guidelines
Our forum has Rules and Guidelines. Please, be kind and read them ;).

Joining the team

Started by Spike, February 20, 2012, 03:09:28 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Spike

If it is of any interest (and if my comeback as developer will finally be made possible) ...

I have yesterday evening ported all my UI changes to the Simutrans Standard codebase - without any translations changes, and only a few header dependency changes in the gui/* code subtree.

A patch will be huge still, and some code constructs will look odd, because the concept of class insulation seems to be new for Simutrans. I like the concept because it reduces compile times, and I do not mind the code oddities that it implies.

I learned it from this book, but I never could convince my codevelopers from the concept while I was project leader for Simutrans. That's why the old Simutrans code has almost nothing of it.

http://www.amazon.com/Large-Scale-Software-Design-John-Lakos/dp/0201633620/ref=sr_1_cc_1?s=aps&ie=UTF8&qid=1329750480&sr=1-1-catcorr

So I think I'll first post a patch or commit my changes to the SVN on sourceforge for the rest of the team members to inspect.

Dwachs

Quote from: Hans on February 20, 2012, 03:09:28 PM
If it is of any interest (and if my comeback as developer will finally be made possible) ...

I have yesterday evening ported all my UI changes to the Simutrans Standard codebase - without any translations changes, and only a few header dependency changes in the gui/* code subtree.

A patch will be huge still, and some code constructs will look odd, because the concept of class insulation seems to be new for Simutrans. I like the concept because it reduces compile times, and I do not mind the code oddities that it implies.
I am definitely interested in having a patch of your changes against standard! Once there is a patch, it can be broken to pieces :D .. to be applied piece by piece.
Parsley, sage, rosemary, and maggikraut.

Spike

I've not yet made a patch, because I'd like to commit the agreed parts by myself into the SVN. I'd like to have my name on the commits (in the SVN logs) and don't want someone else to commit my code under their name.  I know anybody can make a patch from my released code and just do it, and so I can only express my wish hat I'd appreciate it much if I can do the commits with my name on them.

But to do my part of the agreement, I have commited today's code to Sourceforge:

http://simutrans.svn.sourceforge.net/viewvc/simutrans/simutrans/branches/ironbite/

The SVN on Sourceforge says I have changed 437 files, but that is counted against my last Iron Bite release. Compared to Simutrans Standard, there are much less changes - it just shows how much changes I had done in between, and how many now have been reverted by going back to the ST Standard codebase.

Things to discuss - the "total insulation" pattern from the book, I guess, will be a good start for reviewing my new code.

An example of the "total insulation" pattern applied to my new "options" dialog. The header file was free'd of all other header imports, but the base class:

http://simutrans.svn.sourceforge.net/viewvc/simutrans/simutrans/branches/ironbite/gui/optionen.h?revision=799&view=markup

The code now looks like this:

http://simutrans.svn.sourceforge.net/viewvc/simutrans/simutrans/branches/ironbite/gui/optionen.cc?revision=799&view=markup

This pattern has two benefits:

1) If a header file changes, only very few code files must be recompiled.
2) Even for a full compile each code file will compile faster, because there are only few transitive header dependencies.

In case of the UI code, the "private data" class doesn't look too bad. With some other code the pattern might produce less nice results. So I give you this example for inspection, so you can let me know if you like the pattern, or if you don't want it in official Simutrans code.

Code formatting was not changed to comply with ST Standard style yet, but is in my preferred bracing style. I know it must be changed to comply with ST Standard coding rules.

prissi

Well, the code has several different formatting rules, but mostly tries the keep the original prevailing style. But I reality this is a mess and would require automatic reformattig. I even once made a file for a formatter (I forgot which), but then never applied it, to avoid breaking lost of patches (and the experimental branch).

For the UI class insulation seems indeed is not too bad; although it needs to have a new something alloced each time and thus requires an explicit destructor. Well, not a problem for dialoges, which are mostly unique.

For other classes with the possibility of inlining functions I am not so sure that this technique is advantegous, as one trades less compile time (which I personally did not bother much) versus run time (which will bother most people).

Spike

There is no need to apply "total insulation" to everything. This is just the most extreme way of decoupling files. I wanted to show it as example what can be done. One can always have partial insulation, so that some methods can be inlined, or no insulation at all. It is a case-by-case decision, I think.

The options dialog is instantiated/destructed seldom, and does not benefit from inlining. That's why I thought the pattern can be demonstrated sensibly with it.

Dwachs

I could not find a reference to this 'total insulation' idea. Judging from the code you linked, it seems to be closely related to this: (aka pimpl idiom)

http://en.wikipedia.org/wiki/Opaque_pointer
Parsley, sage, rosemary, and maggikraut.

Spike

It's not widely known. This page has a few refrences:

http://www.vincehuston.org/dp/insulation.html

It also explains the difference betwen encapsulation and insulation briefly and referenes Lakos' suggestions in one paragraph.

Also mentioned here, along with the book that I linked a few messages above:

http://c2.com/cgi/wiki?JohnLakos

I don't know why insulation is so little known. It is particular to large C++ codebases, but there should be enough of them so that the suggestions could be of use for a number of projects.

Having said this, I don't mind if you say you don't want it in Simutrans.

To be frank - I don't like discussing things. Lingering discussions put mental stress on me.

I propose ideas which I consider interesting. I'm alright with either a "yes that is good" or a "no we don't want that". I'm not ok with a many day discussion of the pros and cons - if someone cannot understand the benefits of an idea, I'll explain them once or twice. I'll try to be fair enough to also explain the drawbacks. If this is not enough, I'd like a "no", so that the discussion can be ended soon again, even if I am pretty sure the idea is good - but I rather kill a good idea than to suffer from a prolonged and stressful discussion of the idea. (I'd expect that if someone really wants to learn about the idea, go to the local library - if you have, from a universticty of natural sciecne - and read the book. Lakos explains this idea very well.)

Markohs

#7
I respect your decision of not discussing this any further. But I just wanna express I'm interested of hearing the outcome of this, because I'm currently trying to figure how to integrate CEGUI Windows in that part of the code and I can't decide yet of any of my design options, specially because I can't even reuse action_listener_t I think.

If you are so kind to lend me a hand, I'd be happy to explain you the problems that arise on that, Hajo, Dwachs and prissi, and hear your oppinions.

Dwachs

Quote from: Markohs on February 21, 2012, 10:29:11 AM
If you are so kind to lend me a hand, I'd be happy to explain you the problems that arise on that, Hajo, Dwachs and prissi, and hear your oppinions.
Please do.
Parsley, sage, rosemary, and maggikraut.

Spike

#9
Quote from: Dwachs on February 21, 2012, 07:47:35 AM
I could not find a reference to this 'total insulation' idea. Judging from the code you linked, it seems to be closely related to this: (aka pimpl idiom)

http://en.wikipedia.org/wiki/Opaque_pointer

After a second look, yes, that is just another description of the same thing. I didn't know the term "pimpl idiom" though, but "bridge pattern" as it was used in the Design Patterns book.

Edit:

Back on topic - there was a discussion in PMs going on today between Prissi, me and some others of the dev team, and I had to realize that I cannot join the team for several reasons. My health and mental issues don't allow me to, besides some others, but for me the health related stuff is the most important. I can't even participate in a prolonged discussion properly. So this topic can end here, maybe the insulation and coding related talk can be continued elsewhere.

Edit 2: See also:

http://forum.simutrans.com/index.php?topic=9132.msg87604#msg87604

jamespetts

Quote from: Hans on February 21, 2012, 10:09:14 PM
Back on topic - there was a discussion in PMs going on today between Prissi, me and some others of the dev team, and I had to realize that I cannot join the team for several reasons. My health and mental issues don't allow me to, besides some others, but for me the health related stuff is the most important. I can't even participate in a prolonged discussion properly. So this topic can end here, maybe the insulation and coding related talk can be continued elsewhere.

That's very sad. I do wish you the very best for your health. I rather liked your changes to the UI in Iron Bite, and hope that they can become part of Standard/Experimental soon. All of us who play Simutrans will be eternally grateful to your efforts in creating it in the first place, and I very, very much hope that recent bad feeling and disputes never come to overshadow the truly splendid work that you and all of the other contributors to Simutrans have made over the years (15 of them!) since you created the project, and the enormous enjoyment that a great many people get out of it.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Markohs

Quote from: Dwachs on February 21, 2012, 12:05:57 PM
Please do.

Studyng the current code to elaborate a proposal atm, current code is quite complex. I'll post it in the CEGUI thread to keep the conversation in the correct place and not polute this conversation any further.

I used doxygen to generate some documentation to understand it better, with full graphs, but I guess you already have done so too.

Anyway here is the link: http://dl.dropbox.com/u/30024783/Simutrans/doxy/html/annotated.html

isidoro

@Jamespetts: wise words!  I fully agree with you...

Spike

I have some hope again. But I must wait for a reply with the SVN access data. I'm often an anxious person, and envision all sorts of problems until things happen. But now that Prissi says "yes", I have some hope. Prissi and me also agreed to look forward and to try to form a productive team. At the moment it's all brittle and itchy, just growing together, but good will was expressed by Dwachs, Prissi and me. There can still arise protest from the other dev team members though, but since renowned community members also expressed they want to see a joined team, it should be solvable. At least I keep hoping so.