Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use ctrl+click to turn around vehicles in depot is incorrect for short vehicles #4462

Closed
DorpsGek opened this issue Feb 3, 2011 · 7 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Feb 3, 2011

Eddi opened the ticket and wrote:

tested in r21949

when using a grf with shortened vehicles (like early passenger wagons in DBSetXL), and you use ctrl+click to turn them around, they are displayed as if they were a full halftile long (i.e. offset is wrong)

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/4462
@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 3, 2011

Rubidium wrote:

This is (basically) caused by a misfeature/misimplementation of shorter vehicles in TTDPatch and thus OpenTTD. The vehicles are always 8 long, but we let some vehicles start earlier when departing from a depot to give the illusion that they are shorter.

Given the vehicles are (conceptually) drawn from the front facing side, i.e. that is aligned with the 8/8th bounding box. Forcefully reversing vehicles in the depot means that TTDPatch and OpenTTD flip the direction of the vehicle to be drawn. The vehicle can (and I guess should) check the bit whether the vehicle is forcefully reversed and as such should be aligned to the back facing side of the 8/8th bounding box.

So this bug is either:
* a bad implementation of shorter vehicles, and thus duplicate of #3569.
* a bug in the NewGRF.

To properly fix this bug from OpenTTD's point of view, and thus keep NewGRF implementations relatively easy, we need to redo the way shorter vehicles are done which means actually changing the center of the vehicle. However, then there will in odd/8th cases still be a 1/8th offset. That means that we must also disallow odd/8th cases to make this reversing of parts in the depot without any side effects with respect to the vehicle's offsets.

Even then there will be cases where the NewGRF has to actively return different sprites, e.g. the lights of a train. If you add a trailing vehicle with lights and reverse it, should it get lights at the end of the train as well? What with a single engine train with lights? Should red be shown at the front?

So the only sure way to fix this issue is to disallow it from happening at all. Given that removing the feature completely isn't what you'd want, I'd vote for disallowing reversal of short vehicles unless the NewGRF explicitly allows it (and mentions this issue) in which case wrong graphics are a NewGRF bug.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462#comment9578

@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 3, 2011

mb wrote:

The vehicle can (and I guess should) check the bit whether the vehicle is forcefully
reversed and as such should be aligned to the back facing side of the 8/8th bounding box.

Well, "aligning" here means to introduce twice the number of real sprites, which isn´t a good solution, IMO.

If you add a trailing vehicle with lights and reverse it, should it get lights at the end of
the train as well? What with a single engine train with lights? Should red be shown at the
front?

This isn´t a problem, because, other than for wagons, engines may travel in both directions explicitly, and hence provisions would have been made to avoid that "bug" (at least this is the case in DBXL 0.9).

Best thing would be to give the newgrf the right to disallow flipping of vehicles. ATM, this is only possible in rare cases.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462#comment9580

@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 3, 2011

Rubidium wrote:

The main problem with (re)aligning the sprites in OpenTTD is that it will break NewGRFs; different versions of OpenTTD (and TTDPatch) will behave differently w.r.t. the alignment of the sprites. Thus it needs to happen with a new NewGRF version which means making the NewGRF incompatible with TTDPatch (unless someone really wants to fix TTDPatch).

Even with a new NewGRF version it's going to become one huge mess in OpenTTD as we need to maintain support for both the old and new method, which means two ways of setting the right location for the sprites and thus more inconsistencies. Especially when we are going to align from the center; for the odd/8th vehicles we would need to align at a half tile unit, which is impossible with integer math (unless we double all those numbers). So 2/8th and 3/8th would be aligned the same, which means that when (forcefully) reversing the vehicle the image will still be misaligned (although only by 1/8th instead up to of 7/8th of a vehicle length). Nevertheless the issue wouldn't be solved completely.

In what way is changing the white light at the front of a steam engine to a red light handled by a NewGRF? Sounds like it's the NewGRF that needs to have duplicate sprites to get that working right. Or is that the explictly you're talking about?

You can (currently) disallow flipping vehicles (with a bad error message, in OpenTTD) by setting the articulated vehicle callback and then not making an actual articulated vehicle.

In conclusion I think that doing it (correctly) in OpenTTD is too much effort for too little gain. I'd personally vote for disallowing reversal of shortened vehicles unless the NewGRF tells it's okay the reverse the vehicle in which case the graphical bug would be a NewGRF bug (yes, they would need to duplicate the real sprite).
Though that would get inconsistent as you would still be able to turn around unshortened vehicles. However, reversal of articulated vehicles isn't allowed either, although that is done because it would mean swapping vehicles around (getting the articulated bit in front of the main engine). Even then the whole reversal of parts of trains seems like a big mess and I'm not even sure whether the feature is really that useful.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462#comment9581

@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 3, 2011

Eddi wrote:

In that case, i'd vote for generally disallowing turning around vehicles, unless the NewGRF explicitly allows it [by setting some kind of flag].


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462#comment9582

@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 3, 2011

Eddi wrote:

If going the direction of disallowing turning around, one can also think about a new method to turn around articulated vehicles.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462#comment9583

@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 3, 2011

mb wrote:

In what way is changing the white light at the front of a steam engine to a red light handled
by a NewGRF? Sounds like it's the NewGRF that needs to have duplicate sprites to get that
working right.

Well, it can be done easily by recouloring, no duplicate sprites needed (I do it this way).

You can (currently) disallow flipping vehicles (with a bad error message, in OpenTTD) by
setting the articulated vehicle callback and then not making an actual articulated vehicle.

Yes. This is the "explicitly" I was talking of.

I'd personally vote for disallowing reversal of shortened vehicles [...]

I´d second that (see above) but not for locomotives. Game-wise, it makes sense to flip locomotives (if you insist on it), in contrast to wagons.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462#comment9585

@DorpsGek
Copy link
Member Author

DorpsGek commented Feb 4, 2011

Rubidium closed the ticket.

Reason for closing: Fixed

In r21966; or at least, the flipping is disallowed until the NewGRF developer tells its okay. When the graphics are wrong in that case it's a NewGRF bug.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4462

@DorpsGek DorpsGek closed this as completed Feb 4, 2011
@DorpsGek DorpsGek added flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) Vehicles labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant