IRC logs for #openttd on OFTC at 2024-10-15
⏴ go to previous day
00:34:24 *** moll has quit IRC (Ping timeout: 480 seconds)
02:01:53 *** Wormnest has quit IRC (Quit: Leaving)
02:10:39 *** debdog has quit IRC (Ping timeout: 480 seconds)
02:18:07 *** D-HUND is now known as debdog
02:56:49 *** gnu_jj_ has quit IRC (Ping timeout: 480 seconds)
04:59:21 <truebrain> Seems it broke because of GitHub infra issues. Retriggered the build.
05:03:15 <dwfreed> it's hilarious and sad how often Actions breaks because of GitHub internal issues
05:10:39 *** keikoz has quit IRC (Ping timeout: 480 seconds)
06:16:46 <truebrain> It goes worse in the last 2 years or so
06:20:39 <andythenorth[d]> do they use GPT to write their internal infra tools?
06:20:50 <andythenorth[d]> "asking for a friend"
06:25:04 <truebrain> `The promise rejected with the reason "#<Object>".`
08:15:57 *** SigHunter has joined #openttd
08:40:15 *** mindlesstux has joined #openttd
08:41:02 <xarick> wow windows desktop background image was changed
09:04:28 <xarick> so debian linux is woke?
09:06:36 <xarick> i need to stop watching youtube vidoes
09:17:56 <andythenorth[d]> "the times in which we live"
09:25:29 <xarick> I'm tacling TileAdd problem of yesterday
09:25:51 <xarick> seems to be related to overflowing techniques
09:26:51 <xarick> think I found a solution for the x coordinate, but not for the y :/
09:32:16 <peter1138> What is a 'woke' when it comes to a Linux distribution...
09:42:48 <andythenorth[d]> 'whatever you want it to be'
09:43:15 <andythenorth[d]> [regrets incoming]
09:43:22 <andythenorth[d]> I thought all Linux was woke, in the Matrix, red pill sense
09:47:59 <andythenorth[d]> "this will go well"
09:52:33 <ahyangyi> peter1138: Support non-systemd setups
09:59:38 <xarick> doing bit math on signed integers is confusing me
10:00:03 <LordAro> it's generally not a good idea
10:00:20 <LordAro> peter1138: good grief
10:00:35 <LordAro> xarick: depends on the size of the integer
10:00:53 <LordAro> -2 could be 10000001 or 1000000000000001
10:00:55 <xarick> int dx = offset & Map::MaxX();
10:25:41 *** reldred has quit IRC (Quit: User went offline on Discord a while ago)
10:55:51 <johnfranklin> All towns in Sweden can be a city, or all cities in Sweden can be a town?
11:21:52 *** Smedles has joined #openttd
11:28:56 *** brickblock19280 has joined #openttd
11:31:39 <brickblock19280> The only ones that I know have some sort of definition are ort, tätort, stad and storstad where a storstad (probably best translated to city) is a tätort with 250 000 inhabitants
12:42:19 <andythenorth[d]> fishfinger sandwich
13:32:46 <xarick> I created a video showing up all the offsets that are calculated incorrectly in relation to the original TILE
13:35:36 <xarick> the x coordinate is always correct
13:35:47 <xarick> the y coordinate is the problem
13:36:43 <kuhnovic> I'm just amazed that you are wasting your time on this stuff Xarick. If there really was such a big issue with this, then it would have shown itself already in thousands of different ways.
13:37:59 <xarick> correction, the x coordinate is always correct after this fix: `uint32_t x = (TileX(tile_id) + dx) & Map::MaxX();` my bad
13:38:25 <xarick> I don't know how to correct the y
13:40:22 <_glx_> my guess is TileAdd is probably never used on the void tiles border
13:42:48 <_glx_> so the (0, size/2)-(0, size) range triggering asserts for you doesn't happen
13:43:59 <xarick> tileX + dx can underflow
13:44:17 <_glx_> and yes, if I understand the code correctly, only this range seems to be affected
13:44:44 <xarick> but applying (tileX + dx) & Map::MaxX(), it seems to transform the underflowed value into a correct state
13:45:02 <xarick> tried the same approach for y, but that didn't work out
13:46:37 <xarick> the y values are always off by 1
14:16:19 <merni> johnfranklin: I mean, even in English there is no strict definition. The US often calls places with a few thousand people a "city".
14:16:39 <merni> And then there's the UK where "city" is basically a ceremonial title granted for political reasons!?
14:43:21 <kuhnovic> The city of Den Haag (The Hague) here in NL is one of half a million residents and the policital capitol of the country. But technically they never got city rights, so it's just a town.
15:02:33 <xarick> let's test that assert
15:03:44 *** HerzogDeXtEr has joined #openttd
15:10:11 <xarick> that's not where the problem comes from
15:11:06 <ian01223> merni: that's fairly common thing in Europe - in the UK it never really mattered politically (in the past it was something generally given alongside a diocesan cathedral, which was much more important, nowadays it's just a prestige title) but elsewhere in Europe, especially in the Low Countries and the HRE, they were very important up until the 1800s, and gave towns fairly large privileges
15:11:58 <ian01223> a lot of the time in the latter case the towns rights were actually bought by the usually quite rich towns from the princes who controlled them
15:11:58 <xarick> if tile is at 4095,4095 coordinates and I have a x = -4095, y = -4095, the resulting diff is a tile at 0,0
15:17:04 <xarick> those asserts are wrong. You may want something like this if you wanna put asserts there:
15:17:04 <xarick> ``` assert(IsInsideMM(x, -(int)Map::MaxX(), Map::MaxX()));
15:17:04 <xarick> assert(IsInsideMM(y, -(int)Map::MaxY(), Map::MaxY()));```
15:19:42 <kuhnovic> Seems like he realized that too
15:19:44 <Rubidium> xarick, assume SizeX and SizeY being 10. I give you a TileIndexDiff of 9, what did I mean with that? What were the original x and y offsets I passed?
15:24:51 <Rubidium> also, that you were allowed to shoot yourself in the foot doesn't mean shooting yourself in the foot is right. It just meant that no-one thought it to be needed to make a sign/law that shooting yourself in the foot is wrong.
15:26:17 <xarick> TileDiffXY is fine imo, it's the TileAdd that has problems
15:26:57 <xarick> TileDiffXY is correctly giving the diff coordinates
15:28:13 <xarick> TileAdd is the one that needs to ensure it is within bounds
15:28:39 <Rubidium> xarick: please answer the question I just asked...
15:29:14 <xarick> Rubidium: the original x passed is 9, original y is 0
15:29:42 <Rubidium> nope, x = -1 and y = 1
15:31:10 <Rubidium> in this context TileIndexDiff = x + y * 10, so both x = -1, y = 1 and x = 9 and y = 0 give the same result
15:31:59 <Rubidium> and for TileAdd to do its thing, it needs to be able to reverse engineer what the x and y that were passed were
15:33:27 <Rubidium> given there are two solutions, it needs to choose one... TileAdd assumes x to be between -5 and 4 instead of x to be between 0 and 10.
15:34:42 <Rubidium> after all, negative offsets over the X axes are generally way more useful than the full range
15:36:16 <Rubidium> and if TileAdd has that assumption, and it's basically the only function you'd pass TileIndexDiff to, it would be better to ensure the users of TileIndexDiff do not exceed that range of -5 to 4
15:48:53 <peter1138> locosage: hmm, the sprite caching stuff works at grf-encoded level, right?
15:50:24 <peter1138> So to reliability get a composed sprite out for documentation, I need to either bypass the cache, or somehow decompile things... :S
15:56:14 <xarick> rubidium killed my tests
15:57:43 <xarick> i used OpenTTDs own functions to calculate the diffs, and now when I feed the results, it asserts them
16:01:41 <xarick> TileIndexDiffC diff_result = TileIndexToTileIndexDiffC(offset_id, tile);
16:02:02 <xarick> then TileIndexDiff offset = TileDiffXY(diff_result.x, diff_result.y);
16:02:23 <xarick> the new version of TileDiffXY now may assert the value it once gave me
16:05:57 <xarick> offset_id is a tile at (0, 0)
16:06:54 <xarick> diff_result.x is -42, diff_result.y is -80
16:08:30 <LordAro> never ending amount of scoreboard bugs
16:09:10 <xarick> TileDiffXY with the proposed solution asserts. SizeX is 64, SizeY is 128
16:09:18 <peter1138> Could be a News issue, given that's blank.
16:11:38 <peter1138> Okay, the game log has added and removed NewGRFs.
16:11:41 <peter1138> So could just be that.
16:11:55 <peter1138> Given the error is a NewGRF text error, seems likely.
16:12:34 <_glx_> someone didn't read the red window 🙂
16:13:13 <peter1138> Your trace has line numbers but not filenames?
16:17:27 <LordAro> strange newgrf changes though, looks a bit like the exact same newgrfs were readded?
16:18:35 <LordAro> the filenames & versions all seem to match identically
16:19:28 <peter1138> No? It's read top-down.
16:19:34 <peter1138> Added, then removed.
16:19:48 <_glx_> peter1138: ah yes, it's not in the window, nor with ctrl-C, but there's a special copy in right click menu
16:20:26 <_glx_> well not needed in the window, I just click to go on the line 🙂
16:22:58 <_glx_> hmm it seems to be drawing news for STR_NEWS_INDUSTRY_PRODUCTION_DECREASE_SMOOTH
16:23:22 <peter1138> There's an industry newgrf.
16:23:52 <peter1138> The end-game screen is coincidental, it just happens to occur at the same time as a yearly loop...
16:27:21 <Rubidium> xarick: how is TileAdd supposed to differentiate between -42,-80 and 22,-81? Both give you the same TileIndexDiff
16:39:37 <xarick> in release, the resulting tile is still: return tile + offset
16:41:34 <xarick> ```TileIndexDiffC diff_result = TileIndexToTileIndexDiffC(offset_id, tile);
16:41:34 <xarick> TileIndexDiff offset = TileDiffXY(diff_result.x, diff_result.y);
16:41:34 <xarick> assert(offset == ToTileIndexDiff(diff_result));```
16:45:21 *** gelignite has joined #openttd
16:49:19 <xarick> ```TileIndex tile_offset = tile + ToTileIndexDiff(diff_result);
16:49:19 <xarick> assert(tile_offset == tile);```
16:59:23 <xarick> should I try bruteforcing a 4096x4096 map
17:01:17 <peter1138> I guess you could remove TileIndexDiff and always use TileIndexDiffC.
17:06:08 <xarick> for (every tile1 in the map) { for (every tile2 in the map) { run those tests posted above } }
17:06:20 <xarick> and wait for (no) assert
17:08:13 <xarick> offset_id is a tile, I'm just bad at naming things
17:23:51 <locosage> peter1138: Yeah. But make_rgba_image should load it anyway iirc
17:24:15 <locosage> But you'll lose caching of you do it on every build ofc
17:25:32 <locosage> There isn't a good way around it though since cache stores encoded sprite and decoding it back to make an image is a chore
17:32:03 <peter1138> I'm checking if the output image is already there first so that helps.
17:32:32 <peter1138> Although that does mean it needs to be manually deleted if the source image changes.
17:46:11 <locosage> You can try to reuse fingerprints to check for changes
17:46:47 <locosage> I'll check the code to see what can be done when I get time
18:02:21 <xarick> Here's my updated test
18:03:29 <xarick> with decently named variables
18:10:54 <Rubidium> xarick: what's the point you're trying to make there? The whole reason for TileAdd to have the _DEBUG variant is because someone must have been burned by over-/underflows, otherwise there is no reason to add them. What you are "testing" is only cases where there is no overflow, making it completely moot and pointless.
18:14:40 <Rubidium> *you* are the one that assuming that TileAdd and TileDiffXY were written to support both the full range of potential offsets and you are expecting it to be detecting over-/underflows, when the way TileIndexDiff works that is mathematically impossible. Yes, we can chose to store offsets in TileIndexDiff differently, but the whole idea of it is to have super fast addition of it to tiles and with any other
18:14:46 <Rubidium> way of storing offsets you will lose that performance
18:16:27 <Rubidium> the alternative is just removing the debug version, and you can shoot yourself in the foot all day long with undetected overflows and underflows. Have fun with debugging that ;)
18:21:43 *** Wormnest has joined #openttd
19:12:38 <xarick> I have a mad idea! probably bad, but...
19:22:28 <xarick> let me check how often TileAddXY is used, I am testing an alternative
19:30:15 <xarick> almost everywhere TileAddXY is used it x == -1, 0, 1 and y == -1, 0, 1
19:30:41 <xarick> there's a few exceptions
19:31:06 <xarick> GetTileCenter in tilearea_type.h
19:31:58 <xarick> station_gui.cpp is getting station sizes
19:32:23 <xarick> then BeforeAddRect and AfterRemoveRect
19:33:05 <xarick> they are related to station size spread limits, i think
19:33:32 <xarick> and the printdebuginfo in waterregions
19:35:56 <xarick> I suspect the new asserts might cause issues with stations
19:44:54 <peter1138> xarick: REPLY TO THE PR
19:53:19 <xarick> didn't check TileAdd, sometimes it's accessed directly
19:54:20 <johnfranklin> I see peter with all cap letters the first time
20:16:34 *** gelignite has quit IRC (Quit: Stay safe!)
20:20:11 <xarick> the only one that could assert is the follow_track one, i think
20:20:23 <xarick> counting number of skipped tiles on a bridge
20:20:33 <xarick> and bridges can be....... BIIIIIG
20:26:23 <xarick> they can be 64 sized with station spread, pretty sure it would assert the same
20:28:01 <_glx_> if there's no min(spread, mapsize) it's possible yes
20:32:44 <_glx_> or a real bug in callers
20:33:29 <_glx_> and debug version of TileAdd catching it
20:33:57 <xarick> Rubidium: this alternative is starting to sound good 🙂
20:34:43 <xarick> j/k, i just don't know how's this going to be solved
20:35:10 <_glx_> first step would be to check callers don't do silly things
20:36:27 <xarick> release version does a simple tile + offset = new tile
20:37:12 <xarick> but detecting when they're not legit is... not possible apparently
20:38:33 <_glx_> well debug version could validate tile + offset fits in 0-number_of_tiles
20:52:54 <xarick> ```inline TileIndex TileAddXY(TileIndex tile, int x, int y)
20:52:54 <xarick> TileIndexDiffC difference;
20:52:56 <xarick> return AddTileIndexDiffC(tile, difference);
20:52:58 <xarick> ```inline TileIndex AddTileIndexDiffC(TileIndex tile, TileIndexDiffC diff)
20:53:00 <xarick> int x = TileX(tile) + diff.x;
20:53:00 <xarick> int y = TileY(tile) + diff.y;
20:53:02 <xarick> assert((uint)x < Map::SizeX() || (uint)y < Map::SizeY());
20:53:10 <xarick> alternative to TileAddXY
20:54:04 <xarick> avoids calling TileAdd
21:07:11 <xarick> we need a `TileIndexDiffC ToTileIndexDiffC(TileIndexDiff tid)`
21:08:30 <xarick> if we could make the assumption as the rule
21:08:40 <xarick> and enforce it somehow...
21:18:22 <peter1138> Hmm, CCountedPtr is "amusing", it was originally part of YAPF, but isn't used there any more. But it's embedded into the depths of the script API instead.
21:19:38 *** keikoz has quit IRC (Ping timeout: 480 seconds)
21:24:51 *** peter1140 is now known as peter1139
21:36:56 *** nielsm has quit IRC (Ping timeout: 480 seconds)
21:44:03 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
21:51:07 *** tokai|noir has joined #openttd
21:51:07 *** ChanServ sets mode: +v tokai|noir
21:56:02 *** felix has quit IRC (Read error: No route to host)
21:58:06 *** tokai has quit IRC (Ping timeout: 480 seconds)
22:00:23 *** felix has quit IRC (Read error: Connection reset by peer)
22:25:02 *** Flygon has quit IRC (Remote host closed the connection)
continue to next day ⏵
