News:

Use the "Forum Search"
It may help you to find anything in the forum ;).

[BUG] explicitly specifying the non-existence of a cab implies a cab

Started by Mariculous, October 17, 2020, 04:19:18 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Mariculous

Just to make sure it's not me who understood something wrong.
What do the following constraints result in?
constraint[next][0]=Henschel-Wegmann
constraint[next][1]=Henschel-Wegmann_kuppel

has_front_cab=0
has_rear_cab=0
bidirectional=1

Note, there are no prev constraints specified, because anything, including nothing is fine on that side. The latter can obviously only occour when driving "backwards"

I guess we can agree the above should result in a car that can, but does not have to, be the end of a convoy to the "prev" side, whilst not providing a cab. On the "next" side, it has to be coupled with one of the specified vehicles.

The actual result is a car with a cab at the front, which can be coupled to anything.
The same seems to apply to the backside.

Now it gets even weirder!
Commenting out the has_front_cab=0 part will result in exactly what I wanted to define. That is a vehicle which can, but does not have to, be at the end (when driving backwards) but does not have a cab at the front.

I do greatly expect this to be another makeobj error and I'm working on it.
I'd like to get some feedback on wheter I might have missunderstood something here.

RESTRICTED ACCOUNT

Someone seems to have changed that setting recently, can you see if there is a difference in the results before and after that?

Mariculous

If you refer to my commit a few days ago, I fixed a bug which caused has_rear_cab=0; bidirectional=1 to result in a rear cab, but that did not introduce this problem.

I rather assume this is an old issue which was simply yet unknown, because pak128.britain does not specifically set has_*_cab=0 in such cases.

jamespetts

Freahk - can you check whether this issue can be reproduced with an old version that does not include your changes?
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.

RESTRICTED ACCOUNT

Thank you for the details. I think I fixed it. Check pull request # 294.
I would appreciate it if you could check it.

jamespetts

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.

Mariculous

I can definitely confirm it was not introduced by the previous bugfix.

Quote from: Freahk on October 17, 2020, 04:19:18 PMI do greatly expect this to be another makeobj error and I'm working on it.
I'd like to get some feedback on wheter I might have missunderstood something here.
I just fixed it and I'll just throw that work away.
Should not ask for clarification when working on a bugfix. This was a waste of time!

Mariculous

Anyways, thanks for fixing this.
There might be anothr issue in the code, though I could not come up with a case where this actually is an issue. I guess it's simply a missleading naming, but the logic is fine.
The variable can_be_at_front is always true, except from the case where there is an "any" constraint defined.
In fact, a vehicle that has a list of vehicles defined as a constraint, but not "none", also cannot be at the front. Should consider renaming this to something like "prev_has_any"/"next_has_any", obviously swapping true/false in that case.


Further, I am not sure if that's just a weird, but intended definition of can_lead_from_rear=1; bidirectional=1; prev "none", or a bug.
A vehicle that has these three things assigned will have a cab at both sides. Note that it does not matter if there further is a list of vehicles assigned to prev or not.

This is due to the following code:

if (!bidirectional || can_lead_from_rear || has_front_cab) {
basic_constraint_prev |= vehicle_desc_t::can_be_head;
}

Later on, there is the "//auto setting" code that will remove the flag again, if the vehicle is
- unpowered and
- bidirectional and
- cannot lead from rear
Which in our case effectively means never. (as we observe the case can_lead_from_rear=1)

I had just blindly taken this over, but it doesn't seem to make much sense to me. Might be one of the weird but intended implications of can_lead_from_rear, which led to its deprecation.

This time, I'm not working on these things. Just my thoughts I couldn't get out of my head, so I wrote these down.
If you think it's worth fixing, let me know or simply fix it by yourself.

RESTRICTED ACCOUNT

Note that can_lead_from_rear is an obsolete parameter, only left for compatibility.
That is, it was only considered to work to some extent when the cab was undefined in the old dat. Correct operation is not guaranteed.
Because the old spec only clarifies the existence of the cab in the rear, it is "unknown" about the front.
It is not recommended to use it.

Mariculous

I am aware of this.
The logic is still broken in both cases. Not sure if it's worth fixing as it's a deprecated legacy feature though.

RESTRICTED ACCOUNT

It's silly to ask for complete auto-configuration. Manual settings do not need to exist if it is perfect.

Mariculous

Sorry, but I suspect I do not understand that sentence.
What's perfect and what do you mean by a complete auto configuration?