News:

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

Multi-tile attraction buildings can have 4 windows in each direction

Started by Ranran(retired), August 06, 2019, 04:32:55 PM

Previous topic - Next topic

0 Members and 2 Guests are viewing this topic.

Ranran(retired)

Hi, Ranran is here. (´・ω・`)

A multi-tile attraction building can open four dialogs even though it is the same building.

If rotate the camera, it will be treated as a different building and it can have a building dialog for each angle.
On the other hand, factories and stations can only have one window even if they are multi-tile.
This may be a minor issue with standard, but it seems to cause a major issue with extended.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

DrSuperGood

Fortunately one cannot rotate multiplayer games.

This likely stems from map rotation being a hack. It literally rotates the map and everything on it. As opposed to changing the way the graphics are drawn like any decent modern 3D game does.

Why does this mess up extended more than standard?

Ters

In addition, I think one of the tile locations is representative of the entire building, probably the lower-left corner. When rotating, the lower-left corner is at a different coordinate, and also a different corner of the building. I'm not sure which of these two facts matter, but it is probably at least one of them.

prissi

Indeed, the base tile of the corner stores the actual description and handles all action. Since it has a different memory location, it window id is different. Since the map rotation state is known. it could of course check for this too.

jamespetts

The critical code seems to be here (I post the Extended version but the important part of the code is, I believe, unchanged from Standard):


const gebaeude_t* gebaeude_t::get_first_tile() const
{
if (tile)
{
const building_desc_t* const building_desc = tile->get_desc();
const uint8 layout = tile->get_layout();
koord k;
for (k.x = 0; k.x<building_desc->get_x(layout); k.x++) {
for (k.y = 0; k.y<building_desc->get_y(layout); k.y++) {
const building_tile_desc_t *tile = building_desc->get_tile(layout, k.x, k.y);
if (tile == NULL || !tile->has_image()) {
continue;
}
if (grund_t *gr = welt->lookup(get_pos() - get_tile()->get_offset() + k))
{
gebaeude_t* gb;
if (tile->get_desc()->is_signalbox())
{
gb = gr->get_signalbox();
}
else
{
gb = gr->find<gebaeude_t>();
}
if (gb && gb->get_tile() == tile)
{
return gb;
}
}
}
}
}
return this;
}


The current rotation is can be retrieved with welt->get_settings().get_rotation(), and is either 0, 1, 2 or 3, 0 being the initial rotation and the others incrementing once with each rotation until the initial rotation is reached again.

What is not immediately clear to me is the numerical relationship between the position of the tile returned as the first and the rotation. The current code simply seems to iterate through all the tiles in numerical order as stored by the co-ordinates. Quite what alternative algorithm would be needed for any given non-zero value of get_rotation() in order that the same tile be returned whenever get_first_tile be called is unclear, as the geometric relationship between the numerically first tile in that iteration list and what would be the first tile in any given other rotation is also unclear.

Any assistance in this matter would be appreciated.
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.

Leartin

Shot in the blue:
When rotated, it should not be iterated through all tiles the same way. Instead, of
for (k.x = 0; k.x<building_desc->get_x(layout); k.x++) {
for (k.y = 0; k.y<building_desc->get_y(layout); k.y++) {

It would be
for (k.y<building_desc->get_y(layout); k.y = 0; k.y--) {
for (k.x = 0; k.x<building_desc->get_x(layout); k.x++) {

for (k.x<building_desc->get_x(layout); k.x = 0; k.x--) {
for (k.y<building_desc->get_y(layout); k.y = 0; k.y--) {

and
for (k.y = 0; k.y<building_desc->get_y(layout); k.y++) {
for (k.x<building_desc->get_x(layout); k.x = 0; k.x--) {


Therefore, the list of tiles is identical to the list you would have in rotation zero, hence the first tile would always be the same - right?

jamespetts

That is a very interesting idea - thank you. Can anyone assist as to which of these alternative algorithms should match with which rotation states as numerically represented?
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.

Leartin

Disclaimer: I posted (pseudo)code to get across the idea (decrementing instead of incrementing), but I don't even know what all of it means, so it's probably wrong. Perhaps it's so obvious that I don't even need to say this, but don't copypaste it - take the idea and write it yourself.

As for your new question, the one where you decrement both x and y would be rotation 2, you don't even need to know what x and y are and which direction you rotate. Since that leaves you a 50:50 chance, you are probably quicker just going with one option and switching them if it fails the first time, rather than waiting for an answer.

prissi

Changing gebaeude_t::get_first_tile() will lead to all kind of crashes when buildings are deleted or upgraded. The multiple windows do not lead to crashes, so this is a know bug.

Leartin

So was this moved to "solved" because it was somehow solved, despite not stated; or was it moved because it does not matter for standard?

prissi

This is not solved because it is no a real problem. It just means you can open the same dialog four times. No other problems are originating from it, even during deletion. I was not sure where to move, I could have also moved it to denied.

Leartin

See here --> https://forum.simutrans.com/index.php/topic,19150

It's not causing any issues in Standard, but it causes issues in Extended. So I would have expected it to be moved to Extended with a comment like "We won't fix it, do it yourself" or... just leave it open unti James either fixed it or changed the relevant in code in Extended so it no longer needs fixing.

But I'm not complaining, it was just strange to me, so I wanted to point it out. :)


Dwachs

I tried to fix this with r8861. Could you please test?

Is there a pakset with L-shaped buildings or with a building of size 3x3 with empty center tile?
Parsley, sage, rosemary, and maggikraut.

prissi

Yes, we took great care to avoid crashes for such things when the map rotation stated. I think pak64.german had L-shaped mines. I think pak128.German has also empty middletiles (There was a "H" shapes factory you could drive through. Not sure if it went into the set.

With your patch, loading a map which has been rotated, all fatories lose their connections, in pak64 gas stations and junk yards (which are 2x1). I know, why I did not want to touch it. Since it will break games, I have reverted it.

Ok, I will "fixing" this brute force by just iterating over all building tiles to see, if a windows is already open. (done in r8863) If extended does something strange in other corners, then why is this bug report here. There is an extended bug report forum.

Moreover, the multitile citybuilding patch merged with extended is not the one I redid in standard. There were reasons for that, rotation was one of them.

EDIT: It will open a ground window (foundation) instead, but that happens also when clicking twice on 1x1 buildings.

Leartin

Quote from: prissi on November 01, 2019, 12:35:22 PM
If extended does something strange in other corners, then why is this bug report here. There is an extended bug report forum.
Yes, and whenever someone posts a bug that also exists in Standard, they will move it here. The bug is in Standard, it's just irrelevant for Standard. So I can see why nobody would fix it for Standard - all I'm saying is that it would be nice to communicate that to Extended, rather than claiming that the issue does not exist. I just wanted that clarity.

Dwachs

Quote from: prissi on November 01, 2019, 12:35:22 PMWith your patch, loading a map which has been rotated, all fatories lose their connections, in pak64 gas stations and junk yards (which are 2x1). I know, why I did not want to touch it. Since it will break games, I have reverted it.

I could not reproduce this. Did you actually test this? The factory code does not use get_first_tile at all. It is only used for multi-tile attraction and for opening info window.
Parsley, sage, rosemary, and maggikraut.

prissi

Of course I tested it. Since I suspected factories to have troubles I just opened a random dialog, liked at how the coordinates jumped upon rotation and close them games, and restarted it and immediately saw it: Start a game, rotate it three times, and two junkyards still had connections to the consumers (the 2x2 incinerator), but the incinerators no longer listed the junkyards as connected with them. Did not happen before r8861.

Maybe it is only a display problem in the factory dialogue, I did not delve deeper. Just going through all tiles to see if it is already opened much easier (albeit not very elegant, I totally agree with it).

Dwachs

Whatever you saw, any change to get_first_tile cannot affect factories at all. The factory code does not use this method.
Parsley, sage, rosemary, and maggikraut.

Mariculous

I just checked it for extended. There are exactly 12 direct usages of get_first_tile:
stadt_t::update_city_stats_with_building
karte_t::add_building_to_world_list
curiositylist_stats_t::get_unique_attractions
reliefkarte_t::draw
player_t::set_selected_signalbox
get_next_attraction_base (no class in api_world.cc)
signalbox_t
  ::can_add_more_signals
  ::can_add_signal
gebaeude_t
  ::info
  ::is_same_building (2x)
  ::~gebaeude_t

I expect the signalbox stuff to be irrelevant for standard.
At the first view none of these seem to be related to industries. However, I don't know what exacly will happen to the world list on rotation.
add_building_to_world_list makes use of get_first_tile.

prissi

I tested again r8861. I could no longer reproduce the effect of lost connections. So I admid that Dwachs patch seems to work as well, and even fixes the issues with experimental.