Skip to main content ↑ ↓
Topic: Trying to minimize 'silly' differences between experimental and standard (Read 3015 times) previous topic - next topic

Trying to minimize 'silly' differences between experimental and standard

In an attempt to reduce the giant size of the diff between experimental and standard, I am hoping to go through and eliminate 'silly' differences which have crept in -- differences which don't change behavior.  In most cases this would mean putting the standard version into experimental.  But in some cases it would help to be able to make small changes in standard.

-- Translations of comments.  This is the biggest category.

-- Addition of 'sanity checking' code, like ****ertions that pointers are not null, etc.

-- Formatting cleanups and typo corrections.

I wonder if it would be OK if I submitted a series of small patches to eliminate some of these differences.

There are a few other changes which I also consider 'silly' but  I don't know yet what to do about them:
-- a few larger changes which have systematic side effects on code which was essentially unchanged....  mostly in the template cl****es.  For good reasons, welt->get_fab_list() was changed from an slist_tpl to a vector_tpl in experimental; but this causes pointless changes all over the code, even in places where it doesn't really matter.  Is there some way to give vector_tpl a vector_iterator_tpl with the same interface as slist_iterator_tpl?
-- Along the same lines, there's additional functionality in some of the template cl****es in experimental.
-- And experimental added quite a lot of new generic template container cl****es.  I'm not entirely happy with this as I would rather just switch as much as possible over to the STL, but that's apparently not an option yet.  ;)  Perhaps if the interfaces of everything were standardized to match the STL interfaces.... I don't know.

EDIT: Attached is just one patch.  This consists mostly of whitespace and comment improvements, but also includes two actual fixes: one fix to a typo (there was a == where there should have been a =) and one user-visible fix (displaying citycar speeds in km/h rather than unreadable internal numbers).  I would hope to provide many more small patches like this.

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #1 –
Imho, this diff can go into trunk. These additional lines should be deleted in experimental (imho they serve no purpose).

Quote from: neroden – on
-- Translations of comments.  This is the biggest category.

-- Addition of 'sanity checking' code, like ****ertions that pointers are not null, etc.

-- Formatting cleanups and typo corrections.
If you can provide patches for comment translations, they are more than welcome!

I think it is a good idea to let simutrans-standard benefit from the developments in experimental. I myself have no time to follow experimental very closely to pick nice changes there and transfer them to standard.

Quote from: neroden – on
There are a few other changes which I also consider 'silly' but  I don't know yet what to do about them:
-- a few larger changes which have systematic side effects on code which was essentially unchanged....  mostly in the template cl****es.  For good reasons, welt->get_fab_list() was changed from an slist_tpl to a vector_tpl in experimental; but this causes pointless changes all over the code, even in places where it doesn't really matter.  Is there some way to give vector_tpl a vector_iterator_tpl with the same interface as slist_iterator_tpl?
Changing this list to vector_tpl should not create that much overhead. After all a single-linked list of pointers sounds rather pointless.


Quote from: neroden – on
-- Along the same lines, there's additional functionality in some of the template cl****es in experimental.
-- And experimental added quite a lot of new generic template container cl****es.  I'm not entirely happy with this as I would rather just switch as much as possible over to the STL, but that's apparently not an option yet.  ;)  Perhaps if the interfaces of everything were standardized to match the STL interfaces.... I don't know.
Imho our templates can receive some simplifications. In particular, array_tpl, minivec_tpl, vector_tpl are almost the same (minivec and vector uses uint8 and uint32 as index) and could be unified somehow. Infact, array_tpl looks like a subcl**** of vector_tpl.
Parsley, sage, rosemary, and maggikraut.

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #2 –
Quote
Imho our templates can receive some simplifications. In particular, array_tpl, minivec_tpl, vector_tpl are almost the same (minivec and vector uses uint8 and uint32 as index) and could be unified somehow. Infact, array_tpl looks like a subcl**** of vector_tpl.

Actually they have all a different memeory footprint; and more important, array has a fixed size while vector is extensible. But the inbetween changes to vectors with not all elements filled means that vector can indeed take over some functionality from array. (On the other hand, array way mostly used to look up height, and that has gone entirely. Thus nearly nothing from array is left and this can go. Maybe chaning arra2d_tpl to vector2d_tpl would then also make sense.

Minivec was used in dingliste before renovation, thus size was an issue. However, since the remaining places can also do with larger vectors too (even though they do not need them), removing minivec sounds good too.

Converting fab_list into an vector sounds ok too. I think this is more useful, since it can be then easily converted into a weighted_vector_tpl, if you want to play around with p****enger destinations. But instead of making a new iterator for vector, I would rather use the (admitted not very nice) STD iterator, which is available for both.
Quote
            for(  slist_tpl<fabconnection_t *>::iterator i=forbidden_connections.begin();  i!=forbidden_connections.end();  ) {
//            for(  vector_tpl<fabconnection_t *>::iterator i=forbidden_connections.begin();  i!=forbidden_connections.end();  ) {
               fabconnection_t *fc = *i;
               ++i;
            }

And comments need translation (and adding) of course always. (However, care must be taken that SEXP did not change the commented working of the function ...

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #3 –
Quote from: Dwachs – on
Imho, this diff can go into trunk. These additional lines should be deleted in experimental (imho they serve no purpose).
Grrr, I thought I'd stripped out all the blank line additions.  I was going to delete those from experimental.  It can sometimes get confusing remembering which side has the unnecessary blank lines (sometimes it's standard which has blank lines which need to get removed), and I guess I got confused.

EDIT:
Attached is a comment patch for bauer/fabrikbauer.cc (it also has an indentation fix).  This isn't actually exactly what's in experimental; experimental has the German and an English translation side by side.  I improved the translation and removed the German, in keeping with the long-term plan.

EDIT: Also attached is a simple fix for a typo in a comment.
EDIT: And a whitespace addition in vehicle_reader.h where I really do think Experimental is better.
EDIT: And a comment translation patch for besch/roadsign_besch.h, though I'm not 100% sure about this one.

EDIT: Finally, a slightly more complex patch.  This breaks out the check for whether a way is diagonal into its own method (with a very minor change relative to the way the method was broken out in experimental, which I hope to push to experimental), and caches the answer.  Unlike in experimental, it continues to recalculate it every time.  Unlike in experimental, it doesn't use the information for any other purpose (experimental uses it to reduce maintenance costs on diagonal ways).

This should have no user-visible behavior.  There *could* be speed issues; making set_diagonal() 'inline' would fix any speed issues if there are any.

There's a comment fix and a sanity check in the patch too.

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #4 –
For vector iteration, one could use my ITERATE macro, which is just a macro-ised "for" loop for Simutrans container cl****es. There might be some benefit to deploying that in Standard, since it would do away with a lot of unnecessary "for" loop syntax, and reduce the possibility of introducing an error every time that one hand-codes a new one (an error like:

Code: [Select]
for(uint8 i = some_variable; i >= 0; i--)

the avoidance of which was one of the main reasons that I created the macro in the first place.)
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.

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #5 –
There is the STL iterator; I very much want to avoid another one; Using the STL iterator is the best way to go. Macros should be avoided, since they are rarely portable and do not proper typechecking at compile time. Worse they duplicate code or may have side effects.

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #6 –
How would one use the STL iterator for Simutrans container cl****es?
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.

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #7 –
See my post five up ...

The diffs are incorporated. However, I made diagonal part of the flags, since there was anyway space left ...

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #8 –
Quote from: prissi – on
See my post five up ...

The diffs are incorporated. However, I made diagonal part of the flags, since there was anyway space left ...

Thanks!  I didn't think of that.
 

Re: Trying to minimize 'silly' differences between experimental and standard

Reply #9 –
Quote from: prissi – on
See my post five up ...

The diffs are incorporated. However, I made diagonal part of the flags, since there was anyway space left ...

This has now been added to Experimental.
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.