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

[Freahk]

Just to make sure it's not me who understood something wrong.
What do the following constraints result in?

Code: [Select]

constraint[next][0]=Henschel-Wegmann
constraint[next][1]=Henschel-Wegmann_kuppel

has_front_cab=0
has_rear_cab=0
bidirectional=1Note, 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.

[Ranran]

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

[Freahk]

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?

[Ranran]

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

[jamespetts]

Splendid, thank you: now incorporated.

[Freahk]

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

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.

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!

[Freahk]

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:

Code: [Select]

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.

[Ranran]

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.

[Freahk]

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.

[Ranran]

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

[Freahk]

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