[bug 8.x] via Menge does not group cargo with same transfer

[sanna]

In Simutrans Exp. the sort order choice via Menge in the convoy dialogue does not work as in Simutrans Standard; while all passengers and cargo is shown with their transfer point, they are not grouped together according to that transfer point; i.e., their is as many entries as with simply via chosen. This makes the sort choice practically useless...

I have an image... but Simutrans File Sharing wont let me upload it :(

[jamespetts]

Thank you for the report: I suspect that this problem is linked to that reported here, and I hope that the fix that I have applied to solve that problem should also solve this one. Please re-test this when 8.1 is released. Thank you again for your report :-)

[sanna]

The problem does still occur after a fresh git pull of branch 8.x. Was this patch only in trunk?

[yobbobandana]

I just pushed a fix for this to my repo on github . (mesilliac/simutrans-experimental)

[jamespetts]

Yobbobandana,

thank you - very kind! I shall pull your fix and test this evening.

[Tron]

The conflict resolution for freight_list_sorter_t::compare_ware in 36165d2034040c9d7c0e8bb66e6cdd9663c38b70 looks wrong. I suspect this causes the problem.

[jamespetts]

The conflict resolution for freight_list_sorter_t::compare_ware in 36165d2034040c9d7c0e8bb66e6cdd9663c38b70 looks wrong. I suspect this causes the problem.

As long as it's fixed, all is happy!

[Tron]

So you want to unnecessarily diverge from trunk because of a mismerge?
The attached patch should resolve the problem properly.

[jamespetts]

Tron,

is this a patch against the -master branch, or against the -8.x branch? It appears to use the std::sort method that was introduced recently in Standard, and whose introduction I have reverted in the -8.x branch because of the crashes caused on Linux.

[Tron]

The crashes are caused by the mismerge 36165d2034040c9d7c0e8bb66e6cdd9663c38b70, not by the change of using std::sort(). This patch, which corrects the mismerge, applies to the master branch of Simutrans experimental. 36165d2034040c9d7c0e8bb66e6cdd9663c38b70 is contained in master, devel and 8.x. But on 8.x the use of std::sort() was reverted.

[jamespetts]

Tron,

ahh, I see - thank you for the fix, in that case! Because of the subsequent developments, this gets complicated. A couple of things: firstly, does this stand in place of all of yobbobandana's patch/Git commit, or just some of it; and, if so, which bits? Secondly, given that I have also fixed other bugs on the 8.x branch, would it be possible for you to make a patch against 8.x rather than master, so that I can merge cleanly?

Thank you very much for your help on this: it is much appreciated.

[Tron]

I don't see the problem. Just revert the revert (which is trivial, except for this .sdf file - What is this? Should this even be in the repo?) and apply the patch.

[jamespetts]

I'd have to revert the revert manually - isn't creating a .diff with my 8.x branch automatic? I asked because I thought that it'd be quicker for you to produce the .diff than for me to de-revert the changes manually; do correct me if I'm wrong, though :-)

(And the .sdf file probably shouldn't be in the repository - it's a file generated by MSVC++.)

[Tron]

I'd have to revert the revert manually - isn't creating a .diff with my 8.x branch automatic? I asked because I thought that it'd be quicker for you to produce the .diff than for me to de-revert the changes manually; do correct me if I'm wrong, though :-)

I do not know what you mean by "automatically creating a diff with 8.x". I did a commit ontop of master. I can't make heads or tails of the branch structure. E.g. according to the log the mismerge happened when master got merged into devel, but now devel is an ancestor of master. But I'll try: The correction probably should go into devel and The revert should be reverted on 8.x. Then devel (with the correction) should be merged into 8.x:

Code Select Expand


git checkout devel
git am 0001-Correct-mismerge-of-freight_list_sorter_t-compare_wa.patch
git checkout 8.x
git revert 6c65f58bc06cec99dc0546fa7af1cf2cde3de9be
git checkout --ours *.sdf
git merge devel

You probably have to resolve a conflict about *.sdf (or all removed files, see below) again in the last step.

If devel is a dead end, then do this directly on 8.x, i.e. start with revert, then do am afterwards. Or what is master for? It seems to be halfway lost between devel and 8.x. Maybe the correction should be done on master (if devel is dead) and merged to 8.x, but I do not know what your development strategy is.

(And the .sdf file probably shouldn't be in the repository - it's a file generated by MSVC++.)

You should remove it then instead of cluttering the repo with it in every other commit. *.old, *.vcxproj.* and *.vcproj.* probably should go away, too:

Code Select Expand

git checkout devel
git rm *.sdf *.old *.vcproj.* *.vcxproj.*
git commit -m 'Remove client side specific files of MSVC.'
Probably you should do this before correcting the mismerge.
Again, if devel is a dead end, then do this direcly on 8.x.

[yobbobandana]

does this stand in place of all of yobbobandana's patch/Git commit, or just some of it; and, if so, which bits?

As far as I can tell, this is not related to my fix (or this bug) at all. In fact it is related to:
http://forum.simutrans.com/index.php?topic=5227.0

This whole discussion should probably be moved to that topic in stead of this one .

PS:
to stop the MSVC files from being tracked, you'll probably want to use

Code Select Expand


git rm --cached

in stead of

Code Select Expand

git rm
This removes the files from the index without deleting them.
I saw they were added to .gitignore, but they will still be tracked until you remove them from the index .

[jamespetts]

Tron,

the branch structure is thus organised: -master is the branch that contains the code for release versions only. Linux binaries are automatically built every night from the -master branch if it has changed.

-devel is the branch for making significant changes. New features should start life on the -devel branch.

-8.x is the branch for making preparatory pre-releases of release builds in the 8.x series. It is a new addition: previously, there was just -devel (for all development) and -master (for all releases). The idea is that I can make major changes on -devel, but still fix minor bugs on -8.x and release versions with those minor bug fixes whilst the major changes are still in development and unsuitable for release.

In this case, the changes will have been merged in from Standard into the -devel branch. The -devel branch will then have been merged into -8.x, and -8.x will have been merged into -master. Because I am currently working on bug fixes, I have been working on the -8.x branch, so -devel has not been updated since the release of 8.0. -8.x contains a number of different bug fixes, including the reversion of the use of std::sort, so any de-reversion would need to be patched against the -8.x branch rather than against -master.

Tron - perhaps it would be easier if you created a Github repository and pushed the fix to that?

As to the incorrectly staged files: I should leave .vcproj and .vcprojx, as people who develop in Windows might well find those files useful. I shall, however, remove the .sdf file.

[Tron]

Tron,

the branch structure is thus organised: -master is the branch that contains the code for release versions only. Linux binaries are automatically built every night from the -master branch if it has changed.

-devel is the branch for making significant changes. New features should start life on the -devel branch.

-8.x is the branch for making preparatory pre-releases of release builds in the 8.x series. It is a new addition: previously, there was just -devel (for all development) and -master (for all releases). The idea is that I can make major changes on -devel, but still fix minor bugs on -8.x and release versions with those minor bug fixes whilst the major changes are still in development and unsuitable for release.

In this case, the changes will have been merged in from Standard into the -devel branch. The -devel branch will then have been merged into -8.x, and -8.x will have been merged into -master. Because I am currently working on bug fixes, I have been working on the -8.x branch, so -devel has not been updated since the release of 8.0. -8.x contains a number of different bug fixes, including the reversion of the use of std::sort, so any de-reversion would need to be patched against the -8.x branch rather than against -master.

devel should then pick up the correction from 8.x? This smells wrong to me. The mismerge happened on devel, not 8.x.
If you really want to that, then on 8.x revert 6c65f58bc06cec99dc0546fa7af1cf2cde3de9be and am the patch.

Tron - perhaps it would be easier if you created a Github repository and pushed the fix to that?

I wrote down the recipe above.

As to the incorrectly staged files: I should leave .vcproj and .vcprojx, as people who develop in Windows might well find those files useful. I shall, however, remove the .sdf file.

Please re-read my post carefully. I did not say to remove *.vcproj and *.vcxproj. The patterns I gave are *.vcproj.* and *.vcxproj.*, which in particular do not include Simutrans-Experimental.vcproj and Simutrans-Experimental.vcxproj.

[jamespetts]

Tron,

-devel will indeed pick up the correction from 8.x - why does that seem wrong? What would a better workflow be?

As to the recipe - forgive me for being unfamiliar with the Git command line (I use GIT GUI in Windows). What do the following two lines do:

Code Select Expand


git am 0001-Correct-mismerge-of-freight_list_sorter_t-compare_wa.patch


and

Code Select Expand

git revert 6c65f58bc06cec99dc0546fa7af1cf2cde3de9be

? In particular, what effect would reverting the change have if there are other patches applied since the change that still need to be applied in order to fix other bugs?

And thank you for the clarification in relation to the various *.vcproj.* files - in fact, I get an error message when I try to remove those. Thank you for all your help - much appreciated :-)

[sanna]

git am -- applies a patch that you received by  mail (and that was created by git for email sending). git help states "Apply a series of patches from a mailbox"

git revert -- reverts the changes from the stated commit,  leaving subsequent changes in place. Will warn if manual merge will be needed iirc. The git help states "Given one existing commit, revert the change the patch introduces, and record a new commit that records it. This requires your working tree to be clean (no modifications from the HEAD commit)"

[Tron]

Tron,

-devel will indeed pick up the correction from 8.x - why does that seem wrong? What would a better workflow be?

The problem was introduced in devel (when merging master into devel) and later merged into 8.x, so I'd expect that the correction flows in the same direction.

As to the recipe - forgive me for being unfamiliar with the Git command line (I use GIT GUI in Windows). What do the following two lines do:

Code Select Expand


git am 0001-Correct-mismerge-of-freight_list_sorter_t-compare_wa.patch


"am" stands for "apply mail", which turns patches from mails into commits (it can directly operate on a mailbox, but mail formatted patch files, which git format-patch generates, work, too). So this command is like if you edit the files in the patch with all the changes and commit them with message given in the patch file. It also preserves the author and timestamp, so my name would appear in the log instead of yours (you are recorded as committer, of course).

and

Code Select Expand

git revert 6c65f58bc06cec99dc0546fa7af1cf2cde3de9be

? In particular, what effect would reverting the change have if there are other patches applied since the change that still need to be applied in order to fix other bugs?

git revert basically applies the changes in the given commit in reverse. E.g. if in the commit a line was added, revert removes it again. I wonder, how did you revert the sort commit in the first place?

And thank you for the clarification in relation to the various *.vcproj.* files - in fact, I get an error message when I try to remove those. Thank you for all your help - much appreciated :-)

Which error message did you get? The files could not be deleted? This would happen, if MSVC is running and has them opened (The Windows API does not allow removing opened files). You could use git rm --cached, as yobbobandana suggested earlier, which tells git not to track the files any longer, but leaves them in the working copy. I do not know, whether git GUI has a way to do this, too.

[jamespetts]

Tron,

thank you for the instructions - this appears to have worked well, and I have pushed the results to the 8.x branch. Thank you for your help/fix :-)

[Tron]

You removed Simutrans-Experimental.sdf, but what about the other files? You added some patterns to .gitignore, but this has no effect, because the files, which are matched by these patterns, are tracked, so git will not ignore them (except for ipch, which is not tracked).
I think the *.old files should not exist at all, the others should be no longer tracked and ignored. I attached a bundle (I had to suffix it with .dat because of the questionable forum rules), which performs this on 8.x and you can pull from (The bundle cannot express untracking a file while keeping it locally, so you have to restore your user files for MSVC manually).