Passenger list repeats destination

[omikron]

Hello

I've been reading this forum on and off for several years, and played simutrans for just as long, most recently also exp. However, since this issue has never been brought up (to my knowledge), I took the pains in registering in order to ask my question.

When I'm in the passenger lists of stops or convoys, I usually display them in the 'via (amount)' view. Now, in simutrans exp., the first mentioned numbers are large, but then further down the list, the same stops repeatedly show up with fewer and fewer passengers, but repeated massively (see screenshot for example) Is this a bug or something I simply do not understand?

omikron

[jamespetts]

Thank you for the report. This issue is one that I have seen before, and related to the feature in Experimental of storing ware packets' origins. I shall have to look into fixing this when I get a chance, but it is not the top priority.

[omikron]

Hello, I have found the bug and removed it. My lists now list the stops only once. However, I am not sure how to make a patch and I am not sure whether the changes I made make some other problems, therefore I post them in the text here:

In freight_list_sorter.cc the following lines are incomplete:

In line 186 you need to add "|| sort_mode == by_origin" somewhere

Line 160 is the problem. What does it do? If you remove it, the problem is removed.

What do the lines 165-171 do? I do not understand why they are there. In standard, they do not exist.

If somebody tells me how to post code in that scroolbarbox, I will post the code.

omikron

[jamespetts]

Omikron,

thank you very much for investigating this - that is very kind of you. To post code in the scroll-bar box, just put the word "code" in square brackets at the beginning and "/code" in square brackets at the end of the block of code.

[omikron]

Thanks!

lines 152-173

Code Select Expand


if(sort_mode == by_via_sum && pos > 0)
{
//DBG_MESSAGE("freight_list_sorter_t::get_freight_info()","for halt %i check connection",pos);
// only add it, if there is not another thing waiting with the same via but another destination
for(int i = 0; i < pos; i++)
{
if(wlist[i].get_index() == wlist[pos].get_index() &&
wlist[i].get_zwischenziel() == wlist[pos].get_zwischenziel() &&
wlist[i].get_ziel() != wlist[pos].get_ziel())
{
wlist[i].menge += wlist[pos--].menge;
break;
}
else if(wlist[i].get_index() == wlist[pos].get_index() &&
wlist[i].get_ziel() == wlist[pos].get_ziel() &&
wlist[i].get_ziel() == wlist[i].get_zwischenziel())
{
wlist[i].menge += wlist[pos--].menge;
break;
}
}
}


line 160 is the third one starting with 'wlist'
the other IMO superfluous lines are the 'else if' part.

line 186:

Code Select Expand


if((sort_mode == by_name || sort_mode == by_via || sort_mode == by_amount) && pos > 0)


[wlindley]

omikron: Perhaps the easiest way to communicate your changes is to post the output of "git diff" or "svn diff" ... here is what I get after performing your change in the -devel version, for example:

Code Select Expand


diff --git a/freight_list_sorter.cc b/freight_list_sorter.cc
index db70101..cb4f47c 100644
--- a/freight_list_sorter.cc
+++ b/freight_list_sorter.cc
@@ -161,8 +161,8 @@ void freight_list_sorter_t::sort_freight(const vector_tpl<ware_t>* warray, cbuff
                        for(int i = 0; i < pos; i++)
                        {
                                if(wlist[i].get_index() == wlist[pos].get_index() &&
-                                       wlist[i].get_zwischenziel() == wlist[pos].get_zwischenziel()  && 
-                                       wlist[i].get_ziel() != wlist[pos].get_ziel())
+                                       wlist[i].get_zwischenziel() == wlist[pos].get_zwischenziel()
+                                  /* && wlist[i].get_ziel() != wlist[pos].get_ziel() */)
                                {
                                        wlist[i].menge += wlist[pos--].menge;
                                        break;


Such changes can be easily incorporated by testers, with cut-and-paste and a simple merge command.

And yes your change does seem to resolve the problem -- most pleasingly!

[omikron]

I'm sorry if I am a bit thick: How do I find the output of 'git diff' or 'svn diff'?

I'm normally searching archives and libraries for information, and MSVC is a new thing for me. Once I've found out about the basics, I will be able to find my way around on my own, I hope:-)

Regarding the elseif construction: the only use I can think of would be to separate the 'via'-passengers from the 'to'-passengers. Was that the intention? In that case, I will continue to tinker about with this issue until I get it right.

omiikron

[wlindley]

If you are pulling source from the repository, you must have git installed.  Use the command:

Code Select Expand

git diff

That should result in something like the transcript I posted.  You can redirect to a file in the usual way:

Code Select Expand

git diff > mydiffs.txt

[omikron]

Thank you for all the help. I am kind of starting to feel like an idiot....

I lack the basic information: which interface are you talking about? I use MSWindows and the Git Gui and have managed to reach the point where I can see the changes in different colours with +'es and -'es in front of them. However, the only place where I can add any text is the 'Commit message' window, and that won't do me anything.

Where am I lost?

omikron

[jamespetts]

Omikron,

thank you very much indeed for your work on this - very much appreciated. I am preoccupied fixing the server desync crash at present, but will test and hopefully integrate your patch when I get a chance.

Incidentally, to commit, if you are using Git GUI, just click "stage changed" then "commit" after entering a commit message. Then "push" it to your forked repository.

[wlindley]

The commands are for the command-line... you can't script mouse-clicks, and you can't mouse-ahead... but you can type-ahead... I assume the gui version also installs the command-line version?

[jamespetts]

It does indeed - just right click on the folder in Explorer and choose "Git Bash here" rather than "Git GUI here".

[omikron]

I'm still a bit lost, but I found the necessary diff-text, albeit not as a diff, but in the history-window.

This change actually does nothing else than change the part of the code calculating the 'via_sum'-option from standard, slightly translating it to account for the experimental-expressions. It now even separates 'via'-passengers from the 'to'-passengers:

Code Select Expand


---------------------------- freight_list_sorter.cc ----------------------------
index 5abb73f..488bcfa 100644
@@ -156,18 +156,10 @@ void freight_list_sorter_t::sort_freight(const vector_tpl<ware_t>* warray, cbuff
for(int i = 0; i < pos; i++)
{
if(wlist[i].get_index() == wlist[pos].get_index() &&
- wlist[i].get_zwischenziel() == wlist[pos].get_zwischenziel()  && 
- wlist[i].get_ziel() != wlist[pos].get_ziel())
+ wlist[i].get_zwischenziel() == wlist[pos].get_zwischenziel() &&
+ ( wlist[i].get_ziel() == wlist[i].get_zwischenziel() ) == ( wlist[pos].get_ziel() == wlist[pos].get_zwischenziel() )  )
{
wlist[i].menge += wlist[pos--].menge;
- break;
- }
- else if(wlist[i].get_index() == wlist[pos].get_index() &&
- wlist[i].get_ziel() == wlist[pos].get_ziel() && 
- wlist[i].get_ziel() == wlist[i].get_zwischenziel())
- {
- wlist[i].menge += wlist[pos--].menge;
- break;
}
}
}
@@ -183,7 +175,7 @@ void freight_list_sorter_t::sort_freight(const vector_tpl<ware_t>* warray, cbuff
}
}

- if((sort_mode == by_name || sort_mode == by_via || sort_mode == by_amount) && pos > 0)
+ if((sort_mode == by_name || sort_mode == by_via || sort_mode == by_amount || sort_mode == by_origin) && pos > 0)
{
for(int i = 0; i < pos; i++)
{


Edit: Thanks for the tip, James. I'll try that next time...

omikron

[jamespetts]

I have now added this fix to the 10.x branch. Thank you very much for working on it, and apologies for the delay in adopting it!