IRC logs for #openttd on OFTC at 2024-11-06
⏴ go to previous day
02:25:37 *** Wormnest has quit IRC (Quit: Leaving)
03:32:13 *** debdog has quit IRC (Ping timeout: 480 seconds)
04:47:57 <DorpsGek> - Update: Translations from eints (by translators)
05:55:04 *** keikoz has quit IRC (Ping timeout: 480 seconds)
06:00:21 *** Flygon has quit IRC (Remote host closed the connection)
06:05:13 *** toktik has quit IRC (Remote host closed the connection)
07:43:25 *** Guest7922 has quit IRC (Quit: Connection closed for inactivity)
08:57:34 <LordAro> not immediately clear why that failed
09:00:02 <LordAro> 4XX error on uploading macos, i guess
09:18:30 <peter1138> Hmm, 52.1 MiB → 37.8 MiB, reasonable but not that good.
09:23:02 <DorpsGek> LordAro: 0.7255278310940498
09:24:03 <truebrain> LordAro: yeah, uploading of files sometimes fails. And we can't retrigger, as I made it so we aren't allowed to overwrite files 😛
09:24:21 <truebrain> I should look into it, but honestly, it doesn't happen often enough that it bothers me enough to actually make time for it 😛
09:25:32 <peter1138> Ah, + 2.4MiB for the information that was moved.
09:38:20 <LordAro> truebrain: adding --fail-with-body would help a bit for the next time it happens
09:42:34 <truebrain> you know where to make PRs 🙂
09:50:07 <LordAro> idk, i think i've forgotten
10:11:48 <peter1138> Sorry, different size images make it bad :p
10:13:03 <peter1138> Bah, nope, I still can't get the legend to use the samea size.
10:13:10 <xarick> aha, there was a bug in the old code!
10:15:12 <peter1138> Maybe I should run more than 1 tick of the map.
10:16:14 <xarick> The problem was that lakeCenter was being added +1 after it reached the tile it has picked
10:16:34 <peter1138> So this change is moving data that only applies to the consist to a separate structure. Things like orders, unitnumbers, etc.
10:17:06 <xarick> there is no 9204505 that is set to true in that memory of bools
10:17:24 <xarick> but there is a 9204504 which then got a +1
10:17:24 <peter1138> I think someone else had something similar before, but this is making that data optional and using a std::unique_ptr, instead of a whole new pool.
10:22:53 <peter1138> Though this is designed with an accessor in such a way that it could be moved to a pool later.
10:23:08 <peter1138> Saveload of data within a unique_ptr is not a thing though, so that's a bit complicated.
10:25:25 <peter1138> Not proud of this... but it works.
10:36:15 <peter1138> Finding some bugs, though 🙂
10:46:48 *** D-HUND is now known as debdog
10:58:53 <xarick> so it was rubidium's fault, 13 years ago :p
11:01:04 <xarick> but he fixed it 11 years ago anyway, so hmmm it was bugged for 2 years
11:02:23 <xarick> and last year, peter updated it again, no bugs.
11:02:39 <xarick> however, the logic makes little sense
11:03:14 <xarick> so... yeah, I'm inclined to agree that it doesn't need a std::set and that it also doesn't need to be sorted
11:03:16 <LordAro> one wonders how much faster you would be able to work if you didn't have to type out every little thing you find here
11:05:03 <xarick> I always backtrack what I wrote, cus sometimes I forget what I was doing
11:09:32 <xarick> std::vector or std::deque, either is fine
11:09:51 <xarick> i think std:deque is more elegant
11:11:17 *** reldred has quit IRC (Quit: User went offline on Discord a while ago)
11:12:12 <peter1138> count is actually 1 less than marks.size() already.
11:12:36 <peter1138> so count - 1 is excluding 2 values. But because the set is sorted, there's... no reason for that.
11:16:05 <peter1138> I have the feeling the intention was to exclude `begin`, but the code never did that.
11:21:06 <xarick> i think the logic here is trying to find a good spot to make a lake that isn't too small, so it wants a nice flat amount of tiles
11:21:54 <xarick> somehow it was previously count > 10 then changed to count > 32
11:22:04 <xarick> because it was making tiny lakes
11:41:24 <xarick> [2024-11-06 11:41:06] dbg: [misc:0] [CreateRivers] 10859336 us [avg: 10859336.0 us]
11:41:36 <xarick> without all the bloated TicTocs
11:46:47 <xarick> gonna test marks.size()
11:46:58 <xarick> see what it actually builds
11:47:56 <xarick> [CreateRivers] 9686352 us [avg: 9686352.0 us] nice, 1 less second again
11:48:06 <xarick> no unordered_set sorting
11:49:21 <xarick> i went from 23 seconds to 9
11:49:31 <peter1138> Yes, no need you can just use std::size(marks)/marks.size()/whatever coding-style we've not settled on yet.
11:49:57 <peter1138> Was your 23 seconds in debug? Because it was never that slow for me. It does depend on the settings and seed I suppose.
11:50:18 <xarick> it was slow on windows
11:50:26 <xarick> in linux is faster for some unknown reason
11:50:30 <xarick> even as a virtual machine
11:53:27 <peter1138> Different stl implementation perhaps.
11:58:15 <xarick> [CreateRivers] 17840225 us [avg: 17840225.0 us] this is master without the bloat
12:09:20 <peter1138> Yes, because std::set is sorted.
12:09:47 <peter1138> The difference is not wrong, it's just different.
12:21:49 <xarick> I'm too noob for linux
12:21:59 <xarick> how do i run "openttd -d"
12:22:16 <xarick> im already on the right folder
12:25:25 <xarick> 17 seconds on windows, 10 seconds on linux
12:28:09 <peter1138> Self-compiled for Windows, or downloaded?
12:28:31 <peter1138> Windows STL has some extra debugging in some cases which can slow things down.
12:29:33 <xarick> visual studio compiled it
12:46:08 <_glx_> And you set CMAKE_BUILD_TYPE for linux ?
12:48:21 <debdog> xarick: if you run it like "LANG=C openttd -d" the output will be english and we can actually read it
12:48:54 <LordAro> debdog: i'm not sure we need it for that particular example
12:49:12 <debdog> well, for future reference
12:50:26 <LordAro> xarick: to explain, Windows automatically adds your current directory ('.') to your PATH (or where the shell/runtime looks for executables). Linux does not do this, so you need to use an actual path to the executable
12:50:26 <peter1138> Alright, got to 2050 without an assert.
12:59:30 <peter1138> Hmm, I suppose it's only long trains that benefit. Articulated RVs are rare, ships always have a consist. I suppose half of all aircraft don't have a consist either.
12:59:42 <peter1138> ("have a consist" == "have consist information")
13:01:13 <peter1138> Hmm, 1.4GiB → 1.2GiB for Xarick's save.
13:02:29 <peter1138> 599.3MiB down to 424.3MiB + 14.6MiB. Hmm.
13:06:16 <peter1138> "Don't test only worst case"
13:08:42 <truebrain> What are you doing, removing trees? 😛
13:33:16 <peter1138> Clearly making space for more 🙂
13:33:38 <peter1138> Whoops, I doubled memory usage accidentally.
13:56:31 <peter1138> I'm not sure this board can take 128GiB.
14:02:09 <LordAro> nvme as swap partition
14:11:21 <Borg> peter1138: what are you running that you ate all your RAM?
14:12:06 <peter1138> I had a vector that was not cleared between runs. So instead of being under a dozen entries each, it grew and grew.
14:17:44 <Borg> how you would call it that? not clearing a vector between runs?
14:19:37 <LordAro> peter1138: how did you generate those graphs, ooi?
14:23:21 <peter1138> valgrind with massif, and then massif-visualizer
14:24:15 <LordAro> ooh, massif-visualizer was the missing piece of the puzzle
14:24:25 <LordAro> i've tried using ms_print before... impossible
14:25:21 <kuhnovic> Don't you love it when a coworker asks you to review a PR with hundreds of lines of duplicate code copy-pasted from another class, absolutely zero tests, zero documentation or explanation what or why, and the PR is not a draft.
14:25:29 <peter1138> Once it's fixed, this is the result of PR #12345, changing deque to vector.
14:25:50 <LordAro> kuhnovic: "No." *close*
14:26:14 <peter1138> Those deques allocate a LOT of memory.
14:27:26 <kuhnovic> My answer, although a little bit more worded, essentially came down to that yes
14:29:13 <peter1138> std::deque's default size is 512 bytes with my c++ stdlib.
14:29:18 <LordAro> kuhnovic: "Nooooooooo." *close*
14:33:15 *** k-man has quit IRC (Ping timeout: 480 seconds)
14:35:12 <peter1138> Oh that second picture is shit, didn't realise...
14:36:03 <kuhnovic> Is that with a Xarick-savegame?
14:36:14 <peter1138> It is yes. Therefore very extreme.
14:36:30 <peter1138> I didn't use Wentbourne as that doesn't have a path cache when loading.
14:37:00 <kuhnovic> Still, conceptually a vector makes more sense. The memory reduction is a nice bonus.
14:37:07 <peter1138> I was worried about the alignment of RoadVehPathCache being 4+1 = 8 bytes.
14:37:27 <peter1138> But when you realise deque is 512 bytes, twice... well.
14:38:08 <peter1138> I also tweaked it to use a struct in both road and ship path caches, even though ship only has one member.
14:38:25 <peter1138> "it" being this PR.
14:38:51 <peter1138> Reason for that is it makes the saveload code "more similar" and makes changing the contents of the ship patch cache later much easier.
14:40:42 <peter1138> Although I think the Vector saveload change should be separate now, as that needs updating too.
14:42:09 *** pitagoras991_44893 has quit IRC (Quit: User went offline on Discord a while ago)
14:44:30 <kuhnovic> LordAro beat me to the punch, I was thinking the same
14:45:13 <xarick> oh, so deque is bad, vector is good?
14:45:37 <LordAro> it is bad for this usecase
14:45:58 <kuhnovic> The usual answer, "it depends"
14:47:53 *** SigHunter has joined #openttd
14:47:55 <xarick> note to self: stop testing stuff in nonrelease builds!
14:48:02 <kuhnovic> Deque is very nice if you need to add or remove items from the front and back. It also has stable references / iterators, which vector has not.
14:48:11 <kuhnovic> xarick: This is quite the milestone
14:48:28 <xarick> oh, the otherway around
14:48:53 <_jgr_> kuhnovic: You only rarely need both of these things at the same time
14:49:31 <xarick> will I make it to 8 seconds?
14:50:12 <kuhnovic> True. But sometimes std::deque can be somewhat of a nice fix to a vector resizing, ruining it's references in the process
14:50:30 <kuhnovic> xarick: _holds in a yo mama joke_
14:51:05 <xarick> std::advance(cit, marks.size()); was asserting but they were disabled
14:51:19 <xarick> std::advance(cit, marks.size() - 1); was the correct way
14:51:34 <_jgr_> Given that the path caches have fixed length limits, you don't strictly need a variable sized container at all
14:52:40 <kuhnovic> But Xarick, it is really worth it to read up a bit on the differences between different stl containers. They all have their distinct strengths and weakness, and picking the right container can make an enormous difference in terms of performance (both in a good an a bad way)
14:52:44 <xarick> wait a minute, this is also wrong lol i want a random number from marks.size
14:53:49 <kuhnovic> _jgr_: They don't have that anymore, at the ship one doesn't. It just needs to make it to the next water region that's on the high level path, however long or short that path may be.
14:55:19 <peter1138> xarick: You forgot the randomness. marks.size() by itself will of course point to element at the end, which won't work.
14:56:10 <peter1138> The path cache could be fixed size, but then you need to manually manage where you are in it. And ensure it's always big enough.
14:56:30 <LordAro> peter1138: sounds like a vector :p
14:57:12 <peter1138> You can save memory doing that, if you say that the length is less than 256 then you can store offsets in a byte instead of a size_t (8 bytes)
15:01:36 <xarick> std::advance(cit, RandomRange(static_cast<size_t>(std::size(marks)))); it became ugly quickly
15:02:27 <xarick> 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
15:03:07 <LordAro> i think you can figure this one out
15:04:06 <xarick> std::advance(cit, static_cast<size_t>(RandomRange(static_cast<size_t>(std::size(marks)))));
15:04:13 <xarick> still complains... why
15:06:54 <xarick> got it 🙂 im so dumb sometimes
15:07:06 <LordAro> carefully reading error messages is helpful
15:08:00 <_glx_> casting size_t to size_t 🙂
15:08:25 <xarick> i was too used to size_t being the dominant one
15:08:41 <xarick> this time it's uint32_t
15:08:55 <_glx_> static_cast<size_t>(std::size(marks)) <-- this cast is a noop
15:10:53 <LordAro> _glx_: you've got to stop giving him the answers
15:10:59 <LordAro> he's never going to learn
15:14:06 <_glx_> looks like the method is randomly change stuff until it builds
15:15:04 <kuhnovic> Admittedly, out ot all languages I've ever worked with, C++ definitely has the most horrible error messages
15:15:32 <_glx_> some compilers are better for that, but I agree
15:16:04 <LordAro> i have been known to switch between gcc/clang when the error messages get too obscure
15:16:40 <_glx_> but here the error message was quite clear
15:18:06 <_glx_> worst errors are with templates
15:21:45 <kuhnovic> Std::make_unique and std::forward are guaranteed to give you bizarre errors for the most trivial of mistakes
15:22:47 <_jgr_> Rewrite it all in rust? 😛
15:23:13 <_jgr_> (Not that I've learned any of that yet)
15:25:15 <merni> "I'm sure Moore's law will make a 4k map playable in a browser soon enough"
15:25:41 <_jgr_> You can already compile OpenTTD to wasm
15:25:55 <merni> Of course but that's not as funny
15:45:27 <Borg> btw.. was OpenTTD C++ from the begining? I somehow think it was C. or my memory fails me?
16:23:19 <mnhebi> yes, Ludde first decompiled it into C.
16:46:56 <LordAro> "Hint: Although this document is 100% HTML 3.2, it uses nested tables rather heavily. If your browser cannot display it properly, you're probably better off viewing it as plain text, especially if you have a way to filter out all the HTML markup."
17:19:31 <peter1139> Did not expect the refactor to miss things it didn't miss the first time.
17:20:26 <peter1139> I briefly tested with a custom semi-fixed-size container, but it's way more complicated, and didn't save any memory.
17:25:56 <LordAro> peter1139: that was my thought looking at _jgr_'s code - yours looks much nicer
17:29:08 <peter1139> It's possible to put the std::vector behind a std::unique_ptr to make it only 8 bytes if it's unused instead of 24 bytes, but that is extra complexity.
17:29:49 <peter1139> And there's probably some non-standard vector-like library that does waste space by storing size_ts, but... no going there.
17:30:34 <peter1139> Phew, this $work unit test builds now.
18:32:57 <johnfranklin> Why is Peter Peter, meter metRE, center centRE
18:53:55 *** gelignite has joined #openttd
19:04:50 <LordAro> the number of open PRs was making me sad
19:06:10 <talltyler> If you feel like clicking more buttons, #12862 and #12943 could use a look 🙂
19:06:39 <talltyler> The latter turned into shed-painting when all I wanted to do was show the right error text...
19:09:36 <peter1139> Yeah, of course it needed a rebase.
19:10:57 <LordAro> quick, before anyone notices
19:15:27 <LordAro> "Cloning into 'OpenTTD'...
19:15:32 <LordAro> look what you made me do
19:15:45 <talltyler> Who is "you" in this case?
19:29:46 *** osaker has quit IRC (Remote host closed the connection)
19:35:43 <truebrain> LordAro: why didn't you push your commit in the other PR's branch?
19:36:42 <truebrain> does give better contribution to the original author, but owh well 🙂
19:36:55 <LordAro> given i wrote it from scratch...
19:37:06 <truebrain> really? Lol 🙂 That 3 lines \o/
19:37:13 <LordAro> well, i guess i did copy paste the one line they wrote first
19:37:30 <LordAro> but i found the right place, and did *all* the #ifdefs myself :p
19:38:12 *** SigHunter has joined #openttd
19:39:35 <xarick> was my poll invisible or nobody cared to vote
19:45:15 <peter1139> Very savegame bump.
19:46:38 <peter1139> And yes, I realise I messed up the spacing in saveload.h. *Sigh*
19:46:57 <peter1139> VS Code makes resolving conflicts "too easy" so I didn't notice it.
19:48:54 <xarick> with ClearNeighbourNonFloodingStates(tile); inside DoClearSquare, most of ClearNeighbourNonFloodingStates in other places are moot, but meh
19:57:05 <xarick> how do I include std::unordered_set without #include "unordered_set"
19:57:32 <peter1139> You can't, it needs to be included.
20:05:03 <exceptik> talltyler: inb4 openttd gets an option to automatically update currency exchange rates on startup
20:06:24 <LordAro> talltyler: that's why i didn't merge it :p
20:09:08 <LordAro> i mean the whole adding more currencies thing
20:10:07 <exceptik> what if adding a currency would be just a config file with a list of available currencies? 🤔
20:10:21 <talltyler> My reasoning is to reward translators with a custom currency, but 🤷
20:10:38 <LordAro> exceptik: that's called custom currencies, which we already have :p
20:11:05 <exceptik> welp, then this would be an honor thing :)
20:11:59 <talltyler> For what it's worth, PRs don't need to be completely up-to-date to be merged. They just need to not have any conflicts. 🙂
20:13:13 <ketsuban[d]> I figured that was the case. I'm new to this specific workflow, my previous contributions to other codebases were a bit more casual.
20:13:30 <talltyler> No worries, it makes your life easier too 😄
20:14:11 <xarick> `#include "unordered_set"` or `#include <unordered_set>` , what does the code style say about includes? May I also include it just before the function It's being used? or up with all those at the start of the file?
20:15:15 <peter1139> I'm wary of such a change without a bit of analysis.
20:15:35 <_jgr_> xarick: It's a system header, so use the system header syntax. This isn't a local code style issue.
20:17:52 <LordAro> SEE WHAT YOU MADE ME DO?!
20:19:05 <talltyler> Which change are you wary of, Peter?
20:20:24 <LordAro> xarick: based on what you see of every single other include, what do you think?
20:24:28 <xarick> CODINGSTYLE.md surprisingly has nothing about include
20:24:41 <_glx_> because it's a rule of the language
20:24:49 <LordAro> it's not really specific to us
20:25:11 <peter1139> (Not that specifc setting, just the section.)
20:25:49 <LordAro> peter1139: i do feel like it should be split up into multiple commits/PRs
20:25:59 <LordAro> i certainly have no idea what all the changes are doing
20:29:52 <peter1139> I guess frosch's point about 2-way eol still stands.
20:38:21 <xarick> "are you sure you want to squash 14 commits?"
20:44:17 <xarick> historical settings being removed? wowo
20:51:04 <_jgr_> In the new industry production graph, the x-axis seems to deliberately go the opposite way to all the other graphs, it seems slightly odd
20:51:27 <_jgr_> In wallclock mode that is
20:58:52 <xarick> building openttd on linux produces some warnings
20:59:45 *** Wormnest has joined #openttd
21:01:07 *** ChanServ sets mode: +v tokai
21:04:35 <peter1139> VS Code isn't a compiler.
21:05:10 <xarick> hmm how do I find out?
21:06:32 <_jgr_> It'll most likely be gcc, since this is a known gcc false positive warning
21:07:53 *** tokai|noir has quit IRC (Ping timeout: 480 seconds)
21:10:05 <_glx_> but CI does only debug builds
21:11:52 <_glx_> and linux generic (release build) uses 13.2.1
21:12:42 *** NGC3982 has quit IRC (Remote host closed the connection)
21:14:06 *** Borg has quit IRC (Quit: leaving)
21:22:29 *** nielsm has quit IRC (Ping timeout: 480 seconds)
21:24:38 *** gelignite has quit IRC (Quit: Stay safe!)
21:28:15 <xarick> bah... linux openttd is faster
21:30:41 <LordAro> xarick: ring up bill gates
21:45:41 *** HerzogDeXtEr has quit IRC (Read error: Connection reset by peer)
21:49:38 <xarick> uint / int = magic number
22:02:44 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
22:05:03 <xarick> strange, linux is generating a different map than windows
22:05:45 <xarick> i shared the same openttd.cfg
22:06:02 <_glx_> compared master on both too ?
22:07:41 <xarick> does the newgrf check at the start influence randomizer?
22:07:53 <xarick> cus i have no newgrfs installed on the linux machine
22:08:24 <_glx_> ah maybe a different hash function in unsorted_map
22:10:37 <xarick> does that mean unordered_set can't be used for multiplayer?
22:11:20 <xarick> the only other use of unordered_set i can find in the source code is in ship_cmd.cpp
22:11:29 <xarick> searching for nearest depot
22:12:17 <_glx_> it all depends on usage
22:14:01 <peter1139> I was trying to remember that bug we had, but that was a std::map with pointers, so sorted but not deterministic.
22:14:43 <peter1139> That use of std::unordered_set is safe, it does not attempt to iterate it or get anything at a specific 'index'.
22:15:23 <peter1139> If ever FlowRivers becomes part of a Cmd then it would need to be safe as well.
22:16:03 <_glx_> peter1139: was a nice one to trace 🙂
22:16:26 *** keikoz has quit IRC (Ping timeout: 480 seconds)
22:16:35 <xarick> i rather have it generate the same map across all os'es, cus I like AIs to be consistent
22:19:02 <xarick> how do I make a hash function then, if i want to keep unordered_set?
22:20:38 <_jgr_> xarick: You don't want to do this
22:25:44 *** Wormnest has quit IRC (Ping timeout: 480 seconds)
22:26:47 <xarick> changed to std::set on both
22:27:26 <xarick> it's much much slower on windows with std::set
22:29:20 <xarick> does something like stable_unordered_set exist?
22:29:59 <andythenorth> how much faster does my game run with these changes?
22:40:30 <xarick> i asked Mr Copilot about a hash function, he changed my code to include it 🙂
22:54:26 <xarick> i can't understand anything on VSCode...
22:54:26 *** Wormnest has joined #openttd
22:54:35 <xarick> how do I even delete a local branch there?
22:57:19 <xarick> Please clean your repository working tree...
22:59:42 <xarick> i just want to undo the changes ...
22:59:50 <xarick> i dont know how to work with this program
23:00:54 <xarick> undo my local changes :p
23:02:01 <peter1139> There's some good extensions for git for VS Code.
23:05:35 <xarick> finally did it, i had to go to the lines where i made the changes and write back the original text
23:06:16 <_glx_> `git reset --hard` exists for that
23:08:43 <LordAro> peter1139: git restore is the modern-non-checkout command
23:08:53 <peter1138> Or just discard here...
23:10:08 <xarick> copilot gave me a bad hash or ...
23:13:00 <peter1138> `auto cit = marks.begin()`
23:20:01 <xarick> nop, hash approach didn't work
23:20:07 <xarick> still getting different results
23:20:11 <xarick> well off to bed, goodnight
23:46:42 <peter1139> How dare the trains stop in the middle :D
continue to next day ⏵
