News:

Want to praise Simutrans?
Your feedback is important for us ;D.

Signals on left, drive on left

Started by Vladki, March 10, 2015, 08:40:58 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Vladki

Hi everybody,

I have a question about the options signals_on_left and drive_left. What exactly they do with signal images? It seems to me that the position shift is wrong. See the attached pictures (pak128.Britain). When the signals are on the right, they are precisely aligned at the border of tiles. But when moved to left, they get a bit off to the neighbouring tile. And if the signal is on diagonal or slope it is even worse.  Even if signals are realigned for left, like the brand new pak128.britain-ex signals, they still look bad on slopes. And some signs are not affected by these settings at all - railway end-of-choose, min_speed and one-way, and road choose sign.

Perhaps I could have a look at the code if I know where to find this routine.

TurfIt

You're looking for signal_t::calc_image() in obj/signal.cc:49 and roadsign_t::calc_image() in obj/roadsign.cc:212. Interestingly the later starts with the comment "could be still better aligned for drive_left settings ..."

prissi

There is only so much you can do. But the pak128.britain signal you show are too close to the track (sidewards). Hence the automatic realign fails.

Ters

The drive on left doesn't make much sense for railroads. Which side trains drive on is determined by signals and waypoints, not this setting. And there is more to making the signalling left-sided than just shifting the image sideways. For semaphores, the image also need to be flipped. In addition, some countries have road vehicles driving on the right and trains on the left. So maybe it's best to leave the handedness of signals fixed in the graphics, and only affect road signs and vehicles. (Tram signals should proably follow the roads.)

It's also funny that alignment of signals in pak128.Britain is wrong when placed on the left, considering that's where they are supposed to be.

Vladki

The signals in pak128.brit are just modified from pak128. Thanks for the code hint I'll have a look. Btw is there any pak set with drive left or signals on left where the signs and signals are well aligned?

Vladki

I started with fixing the roadsign.cc. I have fixed a few issues but not all:

- choose sign was explicitly displayed always on right. Why?
- private gates are mostly painted in the middle of the tile, so YOFF should be TILE_HEIGHT_STEP/2 on all slopes
- all other offsets (signs and traffic signals) set to XOFF=26, YOFF=13. It may be suboptimal for some paksets, but the important thing is that YOFF = XOFF/2, so that the sign is shown on the other side of the road and not shifted forwards or backwards, to neighbouring tile.

Not fixed:
- bad display on double-height slopes (even when driving on right). There has to be some logic added to double the YOFF for double slopes. Is there some function to find if hang is double or not?
- bad display on single-height slopes when driving on left and slope is south or west. It seems that the yoffsets for front/back image are swapped. There is some code that should do it, but works only for right hand side traffic. I suspect that   

if(  left_offsets  ) {
             hang = ribi_t::rueckwaerts(hang);
}

Does not really do what is expected (swap east/west and north/south)

Anyway, there are some signs that cannot be fixed. To swap properly, all signs should be painted so that thay are on the exit from the tile. But there are signs that are painted to be on the entry (typically one-way signs and motorway signs). As a result they are shifted in opposite direction, far away from the road, or will be broken on slopes and diagonals even for right hand side driving.

Ters

Quote from: Vladki on March 15, 2015, 06:59:03 PM
- choose sign was explicitly displayed always on right. Why?

In pak64 it's a gantry that looks very silly when offset to the left of the track. I think other pak sets have similar designs.

Vladki

#7
Quote from: Ters on March 15, 2015, 07:15:47 PM
In pak64 it's a gantry that looks very silly when offset to the left of the track. I think other pak sets have similar designs.
Is it that bad if the gantry points out of the road? https://www.google.cz/maps/search/charlie+centrum+brno+lisen+cedule/@49.207081,16.692327,3a,75y,90h,90t/data=!3m4!1e1!3m2!1s7lueVur_IaKk-1EJnExo1A!2e0
It is quite confusing for player when this sign is not on the same side as other signs. One is never sure if it is bult correctly.

Anyway, fixes continue:
- railway signs (end-of-choose, one-way sign) were not swapped at all. Now they follow the signals. Offset is same as for road signs.
- railway signals - only fix of offset.

The offsets are a bit problematic. The correct value depends on graphics, and that may be pakset dependent. It is also different for roads and tracks. Tracks are narrower, and the signals are closer to the center. With my test signals, the optimum values were 26/13 for road signs, and 22/11 for rail signals. For now I stick to compromise 24/12, which is not perfect but acceptable. Perhaps these values could be made configurable. Ether for the whole pakset in simuconf.tab or for each signal in dat files.

Any hints for the double height slope detection?

Ters

Quote from: Vladki on March 15, 2015, 09:40:29 PM
Is it that bad if the gantry points out of the road? https://www.google.cz/maps/search/charlie+centrum+brno+lisen+cedule/@49.207081,16.692327,3a,75y,90h,90t/data=!3m4!1e1!3m2!1s7lueVur_IaKk-1EJnExo1A!2e0
It is quite confusing for player when this sign is not on the same side as other signs. One is never sure if it is bult correctly.

That's not a gantry. It's just a sign mounted to the side of the post. (Which is normal for semaphore signals.) But if you take this and offset it ten metres to the left, you get sillyness. (The choose signal in pak64 is missing the left post, but that might be because of some limitation in the engine. There are full gantry images in pak64, but they are not used, and also incomplete.)

Maybe the besch-structures should get a flag indicating whether or not the sign should switch sides depending on drive-on-left. I think there are some symmetric "signs" out there. There is a choose sign or private road sign that looks like a gate if memory serves me right. That doesn't solve the problem of signs that span across the road, but are not symmetric, like overhead traffic lights.

Vladki

There is separate code for private road gates, which does not care about drive on left. That is ok.

I think it is bad idea to check the type of sign in code and decide whether to swap sides or not. It enforces specific designs on pak artists.

An option in pak file to make signals always on right would be great. It would have more uses. Not only for gantry but also for pak.brit maglev (signs are on the track).

Even better would be an option to set the swap offset for each sign.
Non swappable signs would have zero offset.

Ters

Separate images for each would be the ultimate solution, taking care of both offseting and mirroring, but puts more work on the artists.

Vladki

I do not want to add more work to artists, quite the contrary. I think that per-object configurable offset would be enough. Sillyness of some desings could be avoided by offset=0 which would keep the signal the same for both settings. Anyway paks should have recommended driving side. Switching to the other is users choice and may expect some glitches, but I want to minimise those. And that's what the patch does.

Could anyone try it too, or give me some hints on double slope detection in code, and the rueckwaerts function?


TurfIt

I believe you're looking for hang_t::max_diff() for your double height detection. You can see usage examples in brueckenbauer_t::finde_ende().
The call to rueckwaerts looks plain wrong/dangerous - likely accessing outside the rwr[] limits. Perhaps:
hang = ribi_t::rueckwaerts(ribi_typ(hang));

is what someone intended? -adding the ribi_typ() conversion.

Vladki

Hmm, just reading http://forum.simutrans.com/index.php?topic=8789.msg82158#msg82158
It might be more appropriate to use:

uint8 gegenueber(uint8):  gives the opposite slope if the argument is a flat slope, or hang_t::_typ::flach, in other cases

prissi

Signs and signals are to be on the tile border. That is the documentation. If people ignore those, well then the offsets are wrong.

As mentioned above the best way is probably to give the offset in pixel (the other direction is then clear too), i.e. 12 for some of the above mentioned signals. 16 would be the old default and 0 would mean no shift. That way it would work for all signs correctly.

Vladki

Shift 16 (Y-offset) will move the sign from one corner to another. I really doubt that anyone has painted signs and signals in the tile corners. Anyway the most important fix is to make xoff=2*yoff.

I plan to continue on the patch. I just dont have that much spare time.

Vladki

#16
Quote from: TurfIt on March 16, 2015, 09:10:34 PM
I believe you're looking for hang_t::max_diff() for your double height detection. You can see usage examples in brueckenbauer_t::finde_ende().
The call to rueckwaerts looks plain wrong/dangerous - likely accessing outside the rwr[] limits. Perhaps:
hang = ribi_t::rueckwaerts(ribi_typ(hang));

is what someone intended? -adding the ribi_typ() conversion.

Got some time to code. This fix works:


-                       hang = ribi_t::rueckwaerts(hang);
+                       hang = hang_t::gegenueber(hang);


With this fix, signals are ok on half slopes both on right and left. Double slopes seem to require more work.

EDIT: finished the patch - also for double height. Here it is. The diff is against experimental, I'll try to apply it on standard and test.

prissi

I fail to see why the slope should matter. It would only matters when the signal are painted wrongly (i.e. are not at a tile border.)

Vladki

#18
Well, just try it. The signals (even when displayed on the right side), built on slope have to be shifted in Y-axis according to the slope. And this did not work well on double slopes. Some directions were OK, some not. The situation for left hand signals was even worse.

Edit added screenshot of pak64 and pak128, to show that signals on double slopes are wrong.

Ters

In the upper image, it seems that sign facing us on double slope are not offset at all with regards to slope when comparing to the preceding orthogonal edge. The sign facing us on the double slope in the upper and facing away in the lower have the offset they should have for single slope. So the offsets are not totally random, it's just that the logic for selecting the right offset is wrong.

Vladki

And the fix is attached in the previous post. I just made the screenshot to show prissi that the fix is really needed.

Ters

I just immediately assumed that the pictures where of how Simutrans currently looks for you, that would mean the current progress of your patch. But using a ribi function on a hang value makes no sense, and likely produce the "randomness" in the errors you show. (Ribis are edge-based, while hang-values are corner-based.)

I wonder if XOFF and YOFF should be based on tile size. The values look very pak64-ish.

Vladki

The patch is finished and the above bugs are fixed. I just had the feeling that Prissi claims that there's nothing to fix in simutrans and only pak artists are to blame if the sings/signals show up wrong. I tested mostly on pak128.Britain-ex, and XOFF and YOFF are scaled automagically somewhere deeper in the code which I did not touch.
As you can see in the patch I replaceed ribi::rueckwarts with hang::gegenueber, which does what was needed.

jamespetts

Splendid, thank you very much for that. I have now had a chance to test it. For the existing (Pak128 based) signals, it works very well, with the signals in the corners of the tiles as intended. For my new signals, with the offsets deleted, they do not appear quite as intended:



No doubt, this is an issue with the graphics file itself, and I shall have to come up with some new offsets: it is somewhat disappointing that this was not already correctly aligned, however, as it is based on the graphics of the no entry sign, which was correctly aligned (I assume now manually).

In any event, I have committed your changes, which are most helpful. Thank you!
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.

prissi

My apologies. YOu are right, the roadsigns were not double slope enabled.

I still think it would be best to define the offset a signal want to move from right to left (and allow 0 as option for not moving). Will come next.

Vladki

Quote from: jamespetts on March 30, 2015, 08:48:41 PM
No doubt, this is an issue with the graphics file itself, and I shall have to come up with some new offsets: it is somewhat disappointing that this was not already correctly aligned, however, as it is based on the graphics of the no entry sign, which was correctly aligned (I assume now manually).

No entry/one-way signs in pak128.Britain are badly positioned. I plan to do a revision of all (road) signs. Align the signals to the same place as old (pak128) signals, and perhaps adjust them only sidewards (closer/farther from the track).

Quote from: prissi on March 30, 2015, 09:45:19 PM
I still think it would be best to define the offset a signal want to move from right to left (and allow 0 as option for not moving). Will come next.

I agree. I would also allow an option to define negative offset - some one-way signs (motorway entry/exit) facing in the allowed direction are a bit of a hack and displayed on the other side of the road. Yet they are quite common in paks.

Leartin

Quote from: Vladki on March 16, 2015, 07:58:07 PM
I do not want to add more work to artists, quite the contrary. I think that per-object configurable offset would be enough.

I just wanted to sneak in: More options does not mean more work for artists. If there were additional images for left-hand signals, one could just reuse the same graphics with an offset, which is pretty much the same thing as you wanted to do anyway - plus, it is a function artists and their dat-write-slaves already know, so it would be easier to understand then a new parameter.



Vladki

Quote from: prissi on March 30, 2015, 09:45:19 PM
My apologies. YOu are right, the roadsigns were not double slope enabled.

Apologies accepted.

I was about to try my patch on simutrans standard and I see that you already did some fixes but in some other way and only for roadsigns.

Ters

Prissi fixed only the double-height related errors. The original issue is unresolved, since he wants to solve that by giving the pak set artists more control.

Vladki

Quote from: Ters on April 01, 2015, 07:38:26 AM
Prissi fixed only the double-height related errors. The original issue is unresolved, since he wants to solve that by giving the pak set artists more control.

Adding an option for dat files does not interfere with my patch. Just XOFF/YOFF would be read from besch. All other things need to be left as they are.
Prissi's fix does not fix many other issues that I have fixed. It does not fix rail-signals at all, privroad barriers, correct switching for left hand side...
I would prefer to have good display of all signals with fixed offset first. Then we can move on and make more options for pak artists.

Ters

Quote from: Vladki on April 01, 2015, 07:59:09 AM
It does not fix rail-signals at all[...]

Check again.

As for the other things, with a patch fixing multiple things, it might be hard to figure out which changes are related to what problem. For someone familiar with the code, it might have been easier to just fix the reported problems from scratch than figure out what to port from Simutrans Experimental.

Vladki

OK, yesterday evenenig there were changes only in roadsign.cc.
I was just about to make new patch for standard. You were faster. anyway I think this code did not differ too much (only some DE/EN translations).
If you want I can comment the patch more to show what is what.

I'll check the changes again at home in the evening.

jamespetts

Vladki - would there be some advantage in adopting the same approach in Experimental as has been adopted in Standard here?
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.

Ters

I'd say that depends on whether prissi intends to follow up with other changes.

Vladki

Quote from: jamespetts on April 01, 2015, 08:48:19 PM
Vladki - would there be some advantage in adopting the same approach in Experimental as has been adopted in Standard here?

This is gonna be long as prissi's patch is quite different. I will comment my patch an compare it with prissis:

First thing to fix is the basic offsets for left hand signals/signs - bad offset can be seen on flat tiles. Here is the fix for signals, roadsigns and traffic lights:


--- a/obj/signal.cc
+++ b/obj/signal.cc
@@ -132,7 +137,7 @@ void signal_t::calc_image()
                        // signs for left side need other offsets and other front/back order
                        if(  left_swap  ) {
                                const sint16 XOFF = 24;
-                               const sint16 YOFF = 16;
+                               const sint16 YOFF = 12;

                                if(temp_dir&ribi_t::ost) {
                                        image = besch->get_bild_nr(3+state*4+offset);
--- a/obj/roadsign.cc
+++ b/obj/roadsign.cc
@@ -324,7 +332,7 @@ void roadsign_t::calc_image()
                // signs for left side need other offsets and other front/back order
                if(  left_offsets  ) {
                        const sint16 XOFF = 24;
-                       const sint16 YOFF = 16;
+                       const sint16 YOFF = 12;

                        if(temp_dir&ribi_t::ost) {
                                tmp_bild = besch->get_bild_nr(3);
@@ -410,8 +418,8 @@ void roadsign_t::calc_image()

                        // other front/back images for left side ...
                        if(  left_offsets  ) {
-                               const int XOFF=30;
-                               const int YOFF=14;
+                               const int XOFF=24;
+                               const int YOFF=12;

                                if(weg_dir&ribi_t::nord) {
                                        if(weg_dir&ribi_t::ost) {

I know it would be best to have it configurable in dat files, but until I or anyone else does the patch, it should be fixed at least so that XOFF=2*YOFF. Values 24/12 are a good compromise which is not perfect, but acceptable for most signs/signals. Also if it becomes configurable, only one offset shloud be given in dat file and the other calculated. Prissi did not include this patch, but it is imho the simplest and most important.

Then we should take care about half/single slopes. Left hand signals and signs have swapped front/back image and thus need different height shift than right hand sings.
For swapping the direction of slope a ribi_t::rueckwaerts(hang) was used which is a method of class ribi_t not hang_t. Therefore it didnt work right. For hang_t there is a similar funcion hang_t::gegenueber(hang) which does the right thing, i.e. turn the slope 180 degrees.

--- a/obj/roadsign.cc
+++ b/obj/roadsign.cc
@ -290,15 +294,19 @@ void roadsign_t::calc_image()
        else {
                // since the places were switched
                if(  left_offsets  ) {
-                       hang = ribi_t::rueckwaerts(hang);
+                       hang = hang_t::gegenueber(hang);
                }
--- a/obj/signal.cc
+++ b/obj/signal.cc
@@ -102,15 +103,19 @@ void signal_t::calc_image()
                        }
                        else {
                                if(  left_swap  ) {
-                                       hang = ribi_t::rueckwaerts(hang);
+                                       hang = hang_t::gegenueber(hang);
                                }


Prissi is still using the rueckwaerts method, but with proper typesetting, which makes it work right, for both single and double slopes, while gegenueber works only for single slopes (see later)

--- a/obj/signal.cc
+++ b/obj/signal.cc
@@ -55,8 +55,14 @@ void signal_t::calc_image()
        after_yoffset = 0;
        sint8 xoff = 0, yoff = 0;
        const bool left_swap = welt->get_settings().is_signals_left();
+
        grund_t *gr = welt->lookup(get_pos());
        if(gr) {
+
+               const hang_t::typ full_hang = gr->get_weg_hang();
+               const sint8 hang_diff = hang_t::max_diff(full_hang);
+               const ribi_t::ribi hang_dir = ribi_t::rueckwaerts( ribi_typ(full_hang) );
+
                set_flag(obj_t::dirty);

                weg_t *sch = gr->get_weg(besch->get_wtyp()!=tram_wt ? besch->get_wtyp() : track_wt);
@@ -68,22 +74,19 @@ void signal_t::calc_image()
                        }

                        // vertical offset of the signal positions
-                       hang_t::typ hang = gr->get_weg_hang();
-                       if(hang==hang_t::flach) {
+                       if(full_hang==hang_t::flach) {
                                yoff = -gr->get_weg_yoff();
                                after_yoffset = yoff;
                        }
                        else {
-                               if(  left_swap  ) {
-                                       hang = ribi_t::rueckwaerts(hang);
-                               }
-                               if(hang==hang_t::ost ||  hang==hang_t::nord) {
-                                       yoff = -TILE_HEIGHT_STEP;
+                               const ribi_t::ribi test_hang = left_swap ? ribi_t::rueckwaerts(hang_dir) : hang_dir;
+                               if(test_hang==ribi_t::ost ||  test_hang==ribi_t::nord) {
+                                       yoff = -TILE_HEIGHT_STEP*hang_diff;
                                        after_yoffset = 0;
                                }
                                else {
                                        yoff = 0;
-                                       after_yoffset = -TILE_HEIGHT_STEP;
+                                       after_yoffset = -TILE_HEIGHT_STEP*hang_diff;
                                }
                        }


Double slopes. There are two approaches. hang_t has a flag "doppel", and value max_diff. max_diff is calculated using flag doppel, so I thought it would be more efficient to just use the flag directly. Therefore I added a new method to hang_t and slightly modified the method gegenueber to be more readable:


--- a/dataobj/ribi.h
+++ b/dataobj/ribi.h
@@ -69,7 +69,7 @@ public:

        // A little tricky implementation:
        //static bool ist_gegenueber(typ x, typ y) { return ist_einfach(x) && ist_einfach(y) && x + y == 40; }  // unused at prese
nt need to extend to cope with double heights
-       static typ gegenueber(typ x) { return ist_einfach(x) ? (x & 7 ? (40 - x) : (80 - x * 2)) : flach; }
+       static typ gegenueber(typ x) { return ist_einfach(x) ? (ist_doppel(x) ? (80 - x) : (40 - x)) : flach; }
        static typ rotate90(typ x) { return ( ( (x % 3) * 27 ) + ( ( x - (x % 3) ) / 3 ) ); }

        static bool is_all_up(typ x) { return (flags[x] & all_up)>0; }
@@ -82,6 +82,7 @@ public:
        //
        // Ranges werden nicht gepr<FC>ft!
        //
+       static bool ist_doppel(typ x)  { return (flags[x] & doppel) != 0; }
        static bool ist_einfach(typ x) { return (flags[x] & einfach) != 0; }
        static bool ist_wegbar(typ x)  { return (flags[x] & (wegbar_ns | wegbar_ow)) != 0; }
        static bool ist_wegbar_ns(typ x)  { return (flags[x] & wegbar_ns) != 0; }


Then the code can be modifed like this:

--- a/obj/signal.cc
+++ b/obj/signal.cc
@@ -85,6 +85,7 @@ void signal_t::calc_image()
        grund_t *gr = welt->lookup(get_pos());
        if(gr) {
                set_flag(obj_t::dirty);
+               const sint8 height_step = TILE_HEIGHT_STEP << hang_t::ist_doppel(gr->get_weg_hang());

                weg_t *sch = gr->get_weg(besch->get_wtyp()!=tram_wt ? besch->get_wtyp() : track_wt);
                if(sch) {
@@ -102,15 +103,19 @@ void signal_t::calc_image()
                        }
                        else {
                                if(  left_swap  ) {
-                                       hang = ribi_t::rueckwaerts(hang);
+                                       hang = hang_t::gegenueber(hang);
                                }
+
+                               // hang_t::(ost, nord, ...) does not work for double slopes, convert to single
+                               hang = hang >> hang_t::ist_doppel(hang);
+
                                if(hang==hang_t::ost ||  hang==hang_t::nord) {
-                                       yoff = -TILE_HEIGHT_STEP;
+                                       yoff = -height_step;
                                        after_yoffset = 0;
                                }
                                else {
                                        yoff = 0;
-                                       after_yoffset = -TILE_HEIGHT_STEP;
+                                       after_yoffset = -height_step;
                                }
                        }

And the same for roadsign.cc. As you can see, before checking if the slope is north or west, I have to reduce it to single slope. There are no constants for hang_t::doppel_nord or something like that. I also prefer shift right/left instead of multiplication and division by two. So instead of TILE_HEIGHT_STEP * hang_diff I prefer TILE_HEIGHT_STEP << ist_doppel(hang) which does the same but IMHO more efficiently.

Then there is special code for handling private road gates/barriers. These have only backimage, and are mostly painted in the middle of the tile. So if they are on any slope, the yoff shloud be only half of TILE_HEIGHT_STEP:


@@ -271,8 +275,8 @@ void roadsign_t::calc_image()
                set_bild( besch->get_bild_nr(image) );
                set_yoff( 0 );
                if(  hang_t::typ hang = gr->get_weg_hang()  ) {
-                       if(hang==hang_t::west ||  hang==hang_t::nord) {
-                               set_yoff( -TILE_HEIGHT_STEP );
+                       if( hang!=hang_t::flach) {
+                               set_yoff( -(height_step>>1) );
                        }
                }
                else {

Prissi did only cosmetic change to the code but it works as before, i.e. bad.

And the last thing is what signals/signs are affected by signals_on_left and drive_left. All object of type signal (rail, monorail, narrowgauge: signal, presignal, longsignal and choosesignal) are affected by signals_on_left. But there are also rail sings (end-of-choose, one-way, min-speed) which are of type roadsign, but should follow the same side as signals. All road sings should IMHO follow drive_left, whith no exception for choose_sign. Airport signs are mostly painted on runway so I disabled switching for them at all, until we have configurable offset. I was tempted to exclude maglev too (pak128.brit has them on the track). Prissi did not include this fix.

--- a/obj/roadsign.cc
+++ b/obj/roadsign.cc
@@ -257,7 +258,10 @@ void roadsign_t::calc_image()
        after_xoffset = 0;
        after_yoffset = 0;
        sint8 xoff = 0, yoff = 0;
-       const bool left_offsets = (  besch->get_wtyp()==road_wt  &&  !besch->is_choose_sign()  &&  welt->get_settings().is_drive_l
eft()  );
+       const bool left_offsets = ((  besch->get_wtyp()==road_wt  &&  welt->get_settings().is_drive_left()   ) || \
+               (welt->get_settings().is_signals_left() && (besch->get_wtyp()!=road_wt && besch->get_wtyp()!=air_wt)));
+       const sint8 height_step = TILE_HEIGHT_STEP << hang_t::ist_doppel(gr->get_weg_hang());
+

        // private way have also closed/open states
        if(  besch->is_private_way()  ) {

Ideally it would be road signs affected by drive_left, and all others by signals_left.

There is one more thing in the patch from prissi that seems to affect tunnel entrance. However I have checked how signals on tunnel entrances look now, and it seems to need further work...

@ -93,8 +96,7 @@ void signal_t::calc_image()
                        if(  gr->get_typ()==grund_t::tunnelboden  &&  gr->ist_karten_boden()  &&
                                (grund_t::underground_mode==grund_t::ugm_none  ||  (grund_t::underground_mode==grund_t::ugm_level  &&  gr->get_hoehe()<grund_t::underground_level))   ) {
                                // entering tunnel here: hide the image further in if not undergroud/sliced
-                               hang = gr->get_grund_hang();
-                               if(  hang==hang_t::ost  ||  hang==hang_t::nord  ) {
+                               if(hang_dir==ribi_t::ost ||  hang_dir==ribi_t::nord) {
                                        temp_dir &= ~ribi_t::suedwest;
                                }
                                else {


Anyway, please bear with me. I have not coded in C/C++ since my university studies (cca 10 years ago). In the meantime I work as linux sysadmin - so coding only short scripts in bash and sometimes perl. And when I get out of work I prefer to do other things than  computers. But when I get to the computer at home I'm addicted to simutrans, so I felt that improving it would be a good way to start coding again.

prissi

Thank you for you observations. Indeed, for the tunnel entrance there is a mistake.

r7541 has configurable offsets.

Vladki

I have to coorect myself:

Quote
Prissi is still using the rueckwaerts method, but with proper typesetting, which makes it work right, for both single and double slopes, while gegenueber works only for single slopes (see later)

method gegenueber works for both single and double slopes. The further check for direction works only for single slope:

if(hang==hang_t::ost ||  hang==hang_t::nord)

Vladki

Here is a partial patch for experimental for tunnels:


--- a/obj/roadsign.cc
+++ b/obj/roadsign.cc
@@ -318,6 +318,7 @@ void roadsign_t::calc_image()
                        (grund_t::underground_mode==grund_t::ugm_none  ||  (grund_t::underground_mode==grund_t::ugm_level  &&  gr->get_hoehe()<grund_t::underground_level))   ) {
                        // entering tunnel here: hide the image further in if not undergroud/sliced
                        hang = gr->get_grund_hang();
+                       hang = hang >> hang_t::ist_doppel(hang);
                        if(  hang==hang_t::ost  ||  hang==hang_t::nord  ) {
                                temp_dir &= ~ribi_t::suedwest;
                        }


I had it prepared to go, but wanted to test the tunnel entrances a bit more while Prissi had chosen diifferent approach:

@@ -275,8 +277,8 @@ void roadsign_t::calc_image()
                if(  gr->get_typ()==grund_t::tunnelboden  &&  gr->ist_karten_boden()  &&
                        (grund_t::underground_mode==grund_t::ugm_none  ||  (grund_t::underground_mode==grund_t::ugm_level  &&  gr->get_hoehe()<grund_t::underground_level))   ) {
                        // entering tunnel here: hide the image further in if not undergroud/sliced
-                       hang = gr->get_grund_hang();
-                       if(  hang==hang_t::ost  ||  hang==hang_t::nord  ) {
+                       const ribi_t::ribi tunnel_hang_dir = ribi_t::rueckwaerts( ribi_typ(gr->get_grund_hang()) );
+                       if(  tunnel_hang_dir==ribi_t::ost ||  tunnel_hang_dir==ribi_t::nord  ) {
                                temp_dir &= ~ribi_t::suedwest;
                        }
                        else {


Anyway there is further bug, prehaps related to the code a few lines after the place where this patch is applied. There is check for IMG_LEER and in some circumstances the front image is drawn as backimage and vice versa. It seems to be dealing with signals and stations on the same tile, but it works OK only for Nort-South direction. East-West is broken for both stations and tunnel entrances - the signal is drawn first, and then the station/tunnel graphics covers the signal. On the attached screenshots, there are two way signals on statins and tunnel entrances, but you can see some missing. For signals_on_left, the bug manifests in North-south direction, and east-west is OK.

prissi

That is how it is inteded to look. Otherwise the signal would be on top of the station. Rather a station with a close roof is not recommended for various reasopn.

Vladki

Quote from: prissi on April 06, 2015, 09:54:36 PM
That is how it is inteded to look. Otherwise the signal would be on top of the station. Rather a station with a close roof is not recommended for various reasopn.

The pictures you see are rotations of the same station. It has signals on both sides, and the tunnell too.  And yet you can see that the signal for driving east disappears. I really doubt it is intended.

I chose this station to show how it covers the signal as it was similar to how it is covered by tunnel entrance.

Vladki

Quote from: prissi on April 03, 2015, 09:47:46 PM
r7541 has configurable offsets.

I have tried it, but almost no roadsigns get moved to the left side. Here is a patch:


diff --git a/besch/reader/roadsign_reader.cc b/besch/reader/roadsign_reader.cc
index 1d18dcc..0fc071d 100644
--- a/besch/reader/roadsign_reader.cc
+++ b/besch/reader/roadsign_reader.cc
@@ -87,7 +87,7 @@ obj_besch_t * roadsign_reader_t::read_node(FILE *fp, obj_node_info_t &node)
                dbg->fatal("roadsign_reader_t::read_node()","version 0 not supported. File corrupt?");
        }

-       if(  version<=3  &&  (besch->is_choose_sign()  ||  (besch->flags & ~roadsign_besch_t::PRIVATE_ROAD) == 0)  &&  besch->get_waytype() == road_wt  ) {
+       if(  version<=3  &&  ((besch->is_choose_sign()  ||  besch->is_private_way())  &&  besch->get_waytype() == road_wt  )) {
                // do not shift these signs to the left for compatibility
                besch->offset_left = 0;
        }


prissi

Thank you; with && the second set of () are not needed.

prissi

The hiding of signals in the east facing tunnels is a fundamental limitation with no easy way out. Even making this a foreground image does not solve the issue, since the tunnel (or station) is draw later to prevent other issues.

Vladki

Hi, I'm reopening this topic because part of the original patch was overlooked (or not incorporated for any reason). It deals with private road/rail barriers/gates on slopes. Their position was not corresponding to the slope - most of the graphics for gates are in the middle of tile, but it was displayed on height corresponding to the south or east border of tile. Anyway here is the new patch:


Index: obj/roadsign.cc
===================================================================
--- obj/roadsign.cc     (revision 7727)
+++ obj/roadsign.cc     (working copy)
@@ -243,9 +243,7 @@
                set_bild( besch->get_bild_nr(image) );
                set_yoff( 0 );
                if(  hang_diff  ) {
-                       if(hang_dir==ribi_t::west ||  hang_dir==ribi_t::nord) {
-                               set_yoff( -TILE_HEIGHT_STEP );
-                       }
+                       set_yoff( -TILE_HEIGHT_STEP*hang_diff/2 );
                }
                else {
                        set_yoff( -gr->get_weg_yoff() );


The same problem is in experimental, though the patch may need some adjustment.

prissi

Well, a valid argument. Incorporated in -r7728. THanks