IRC logs for #openttd on OFTC at 2024-01-10
⏴ go to previous day
00:08:54 <_glx_> yeah happens with huge PRs 🙂
00:09:21 <_jgr_> I probably should have looked at it earlier
00:10:48 <_jgr_> It seems pretty nice, from all that I can see
00:10:50 <truebrain> Would that have helped? 😄 easy to miss ofc 🙂
00:11:33 <_jgr_> The saveload stuff is confusing me a little, but I think I need to spend some more time looking at it
00:12:26 <truebrain> And otherwise poke the author 🙂
00:13:01 <truebrain> He seems more than willing to explain and help, given he even tries to help X 😄
00:59:04 *** Wormnest_ has quit IRC (Ping timeout: 480 seconds)
01:35:13 *** Wormnest_ has joined #openttd
02:07:15 *** Wormnest_ has quit IRC (Quit: Leaving)
03:51:41 *** debdog has quit IRC (Ping timeout: 480 seconds)
04:06:08 <reldred> I thought he was banned from github?
04:07:11 *** kiothemodder has joined #openttd
04:07:11 <kiothemodder> i not know what he did but change in link look minor and normal
04:07:53 <reldred> He's fine usually, but historically has thrown some absolutely catastrophic tantrums. He's still banned from the discord.
04:09:10 <kiothemodder> oh that guy? i see. i forgot about him. i heard he say he got drugged bad by bad doctor?
04:09:32 <reldred> Oh he has lots of different excuses.
04:10:14 <kiothemodder> i not think that excuse. he act exactly like my unkle when meds were bad
04:10:36 <kiothemodder> but then i not know whole story
04:11:14 *** yorks_andy has joined #openttd
04:11:20 <emperorjake> He has a long history of acting out, under 3 separate accounts too
04:12:22 <emperorjake> Fastest spambot ban ever
04:15:18 <kiothemodder> well you can say what you want and think what you want. you know more that me. i do know my uncle never get forgive for thing not his fault even when papa stick up for him and now life ruined many years. anyway i come to suggest feature.
04:16:35 <kiothemodder> make possible to count number of tile since last station and make vehicle break down as option to simulate fuel?
04:16:36 <belajalilija> gadg8eer mentioned
04:17:06 <kiothemodder> i not mention him or want talk about that
04:18:15 <kiothemodder> reldred mention first because commit
04:18:34 <emperorjake> kiothemodder: So that would be like the breakdown mechanic but more predictable?
04:23:28 <kiothemodder> need new option to put how many tile until fuel run out but can be done?
04:24:14 <kiothemodder> ```vehiclefuel_tiles: 1;``` in nml?
04:24:31 <Eddi|zuHause> no, there is no fuel
04:24:59 <kiothemodder> i ask if can be put into game
04:25:10 <Eddi|zuHause> no, there is no fuel
04:25:38 <kiothemodder> i mean as game patch
04:26:07 <kiothemodder> i know not yet possible in nml
04:26:11 <Eddi|zuHause> no, there is no fuel
04:26:45 <kiothemodder> help? why is bot repeating thing?
04:27:25 <belajalilija> Fuel sounds like a bad feature tbh
04:28:25 <belajalilija> Opt in with different settings maybe
04:28:53 <belajalilija> Av8 has range limits for aircraft but they suck on bigger maps
04:29:35 <kiothemodder> yes would need scaling for map
04:30:06 <kiothemodder> i not sure but one tile two bus long?
04:30:59 <kiothemodder> maybe use tile length of two bus to tell how far tile is without real scale?
04:31:46 <belajalilija> Theres no scale but i think generally people treat 1 tile as 50m
04:32:32 <belajalilija> Most modern carriages are about 26m
04:32:53 <belajalilija> Whereas busses are about 10-12
04:34:36 <kiothemodder> ok i think that make good referee
04:35:18 <belajalilija> 50m is significantly more than 125ft
04:35:47 <belajalilija> Probably more like 170
04:39:39 <kiothemodder> ok ~50m / ~165 ft is guideline then and tile value should have factor property with 0,xx for vehicle set maker to decide?
04:40:45 <belajalilija> You’re talking code stuff now
04:40:55 <belajalilija> Which is not something i know
04:41:50 <kiothemodder> ok thank you for try
04:41:50 <kiothemodder> does feature sound good if optional and disable by default at least?
04:42:19 <kiothemodder> should i suggest to jgr or main?
04:43:03 <belajalilija> Maybe have a train needing refuelling every 3/4 of whatever the shortest map edge is
04:43:18 <belajalilija> And 1/4 for road vehicles
04:43:54 <belajalilija> Wouldn’t know where best to suggest it
06:12:56 *** keikoz has quit IRC (Ping timeout: 480 seconds)
06:36:22 *** asymptotically2 has quit IRC (Write error: connection closed)
06:36:58 *** SuperAdmin has joined #openttd
06:38:58 *** asymptotically2 has joined #openttd
06:42:31 *** asymptotically2 has quit IRC (Read error: Connection reset by peer)
06:44:36 *** SuperAdmin has quit IRC (Quit: Page closed)
07:31:36 *** tokai|noir has joined #openttd
07:31:36 *** ChanServ sets mode: +v tokai|noir
07:38:15 *** tokai has quit IRC (Ping timeout: 480 seconds)
07:54:21 *** asymptotically2 has joined #openttd
08:04:11 *** asymptotically2 has quit IRC (Read error: Connection reset by peer)
08:12:52 *** D-HUND is now known as debdog
08:15:06 *** asymptotically2 has joined #openttd
08:34:00 *** asymptotically2 has quit IRC (Read error: Connection reset by peer)
08:43:07 *** asymptotically2 has joined #openttd
08:43:36 *** asymptotically2 has quit IRC (Remote host closed the connection)
08:43:56 *** asymptotically2 has joined #openttd
08:48:04 *** asymptotically2 has quit IRC (Read error: Connection reset by peer)
08:55:20 *** asymptotically2 has joined #openttd
09:03:23 *** asymptotically2 has quit IRC (Ping timeout: 480 seconds)
09:52:45 <orudge> So our expenses last year were just £26 than in 2022; donation income was £1,202 more though, so we only lost around £1100 last year compared to £2300 the year before. If we never get another donation again, we've got enough money to keep us going until September.
09:52:58 <orudge> However we've already had £80 of donations this month, so that hopefully won't be an issue either.
09:53:12 <orudge> *just £26 more than in 2022
10:04:11 *** alfagamma7 has joined #openttd
10:04:31 <alfagamma7> OpenTTD is a volunteer driven project after all
10:06:25 *** asymptotically2 has joined #openttd
10:06:59 <peter1138[d]> I can't believe andy's MBP costs so much to run.
10:08:58 <alfagamma7> I wonder whether running the forums website costs a lot
10:09:56 <orudge> It doesn't cost as much as OpenTTD itself, for sure :)
10:11:02 <alfagamma7> If I ever get to donate , I probably would have a hard time doing so since it costs a lot in local currency
10:11:26 <orudge> No problem, nobody is obliged to donate of course
10:12:02 *** asymptotically2 has quit IRC (Read error: Connection reset by peer)
10:22:51 <peter1138[d]> The donation details for GBP Bank Transfer are out of date, you generally need Account Name these days as well.
10:29:29 <orudge> Ah, true, though I think I only had one person contact me to query that. I've added it into the forums and will do so for OpenTTD later then.
10:41:20 <_zephyris> Thank you orudge for coordinating all of this!
10:50:05 <kuhnovic> Hey guys, I've been thinking / testing a bit. If the user orders a ship to go to a depot, it will look for the closest depot without any distance restriction. Previously the ship finder would throw in the towel if the depot was too far away, but with the new pathfinder it will be able to find depots even on the other side of the map. I think we should limit the depot search to a reasonable
10:50:05 <kuhnovic> distance. What do you guys think?
10:52:16 <peter1138[d]> Now define reasonable 🙂
10:52:27 <kuhnovic> This doesn't apply to automatic servicing btw. That uses a distance which is based on the PF settings. The calculation is a bit odd IMO, but at least it's limited. The default values lead to a search distance of 20 tiles.
10:52:42 <kuhnovic> peter1138[d]: Haha yeah that's the hard part
10:53:12 <peter1138[d]> By "orders" you mean press the the "go to depot" button, rather than depot orders, I assume.
10:54:32 <peter1138[d]> So does it search all depots to find the shortest path?
10:54:33 <kuhnovic> Yes. Bad choice of words haha.
10:55:30 <kuhnovic> It iterates over all depots and picks the one with the shortest manhattan distance to the ship. The PF isn't called.
10:56:31 <kuhnovic> This also means that the ship could target a depot that it can't reach at all. But that's a different issue, something xarick is looking into.
10:57:10 <peter1138[d]> Ah so manhattan distance not path distance.
10:57:53 <kuhnovic> Nope. Just as the crow flies... if the crow were to fly only along the x or y axis 😛
10:58:18 <peter1138[d]> Is it possible to just search water regions?
10:58:37 <kuhnovic> Yes. That's something I'm looking into as well.
10:59:20 <peter1138[d]> If WaterRegion had a CompanyMask of depots, you could perhaps do a high-level pathfind.
10:59:30 <peter1138[d]> But I don't know if that's feasible to maintain.
10:59:44 <peter1138[d]> Depots don't change too often (unless it's Xarick's AI)
11:00:16 <peter1138[d]> (I also don't know if that makes sense at all, I don't know that much about your system yet :))
11:01:02 <xarick> the way it is now, it needs to have the destination already defined upfront
11:02:17 <xarick> search for a depot doesn't have that
11:02:22 <kuhnovic> That's if you use the ship pathfinder to find the depots. There are other, more efficient ways.
11:03:52 <xarick> I'm experimenting repurposing tile_patch_labels to also mark which of the local tiles have depots
11:03:58 <kuhnovic> But regardless of how we achieve this, I think we need to limit the search distance. Even if the user pressed the go-to-depot button.
11:06:00 <xarick> another issue is the matter of costs. how exactly, within the same patch label, calculate the one which is quicker to walk to
11:08:42 <kuhnovic> Yup, that's quite an annoying issue. The water regions provide a position resolution that is just too low. If a ship and a depot are 1 water region apart, then they can be somewhere between 2 and 32 tiles apart from each other...
11:19:05 <xarick> Another option to consider - use the low level pf for searching closest depots with the old 10000 max nodes, the easy way out. If the search has a defined max penalty (service at nearest depot), that's just about 20 tiles distance apart, or 2 regions worth of distances, there's not much to gain using the high pf.
11:21:42 <kuhnovic> That would be an option, but that would mean you'd have to add / maintain another YAPF class with specific origin / destination implementations. Blegh.
11:22:13 <kuhnovic> I hate that CRTP implemenation of YAPF 😛
11:23:29 <xarick> But with an unlimited penalty (manually send to depot / go to nearest depot order), ... yeah... this is where high pf would help.
11:24:50 <kuhnovic> My suggestion, and the approach that I'm currently looking to:
11:24:50 <kuhnovic> 1. find a blob of reachable water region patches within a certain distance of the starting tile (the ship's position).
11:24:50 <kuhnovic> 2. find all the depots that are within those reachable patches
11:24:50 <kuhnovic> 3. determine which one is closest. Just use manhattan distance, although I think euclidian is more fair here.
11:26:53 <xarick> instead of manhattan distance, could it be possible to know from which tile position at the edge that region is accessed from?
11:26:54 <kuhnovic> It's essentially the same as what we have already, except that we also check if we can actually reach the depot. Which is not entirely unimportant
11:28:34 <peter1138[d]> kuhnovic: If you have other ideas on how to implement the pathfinder... I don't think anybody is really attached to how YAPF is designed, most of us won't touch it with a barge-pole!
11:28:51 <peter1138[d]> (Says I, the person who implemented CRTP for dropdown lists...)
11:29:31 <kuhnovic> I'm not sure if I'm mad enough to start tearing up the YAPF implementation just yet 😛
11:29:33 <xarick> if we know which tile the area is entered from, we could perhaps do a low level pf search from there to the depot location within the area, or to the next area... do these many mini low lvl calls to extract the (hopefully) exact costs from travelling it
11:30:44 <kuhnovic> xarick: IMO these kind of approaches may work to some extent, but you quickly end up in edge cases (pun intended). And things tend to get complicated really quickly.
11:32:35 <kuhnovic> I'd much rather go for something that's relatively simple and works most of the time, even though there is an opportunity to create a pathological scenario where it wouldn't choose the most optimal depot.
11:34:05 <xarick> hmm the pathological scenario of ship undecided whether to service or not service
11:35:04 <peter1138[d]> It'll delay searching for a day? 😉
11:36:10 <xarick> no, this is what the issue reported
11:36:16 <xarick> forgot the number of the issue
11:37:40 <kuhnovic> IMO that issue is relatively easy to fix using the regions
11:38:32 <xarick> rather just not use the regions in my opinion, as long as the penalty is that limited
11:38:41 <xarick> go straigth to using the low lvl pf
11:38:57 <merni> kuhnovic: Does it really matter if the depot is not *optimal*, as long as it's reasonable? eg. I don't really care if the ship goes to a depot 20 tiles away rather than one 5 tiles away
11:39:23 <merni> but rather that it doesn't go to something on the other side of an ocean or tries to head to an inaccessible depot
11:40:18 <merni> peter1138[d]: In that sense this method seems to be suitable though of course maintaining depot info in each region might be more difficult
11:41:12 <kuhnovic> On longer paths I agree that a little suboptimality is perfectly fine. But it does look odd if there is a depot 2 tiles away but instead it picks one that is 20 tiles away...
11:41:25 <merni> I've had that happen with trains anyway
11:41:54 <merni> even if there is a closer depot on one branch trains may decide to take another branch for a somewhat further one
11:42:00 <peter1138[d]> My thought was use high-level pathfinder to find suitable navigable depots, then use low-level pathfinder to find the shortest path.
11:42:04 <peter1138[d]> Might be too much.
11:42:51 <merni> the "send to depot" button is not for frequent use anyway, shouldn't be too much of an issue
11:43:26 <peter1138[d]> merni: depends how water regions are updated -- adding/removing seems simple enough if the depot can find what region it is in. Updating regions may require some other heuristic -- or a rescan of all depots for brute-force lazyness.
11:43:50 <peter1138[d]> Well there's also ship-needs-servicing depot finding.
11:49:20 <peter1138[d]> Although if the high-level path finder is quick, and doesn't find a depot in a "reasonable" distance, then it seems fine.
11:49:38 <peter1138[d]> Again, defining reasonable distance is fun 🙂
11:50:42 <peter1138[d]> Weird, that CD ripped really quickly.
11:51:54 <merni> peter1138[d]: Oh right, forgot that was a thing
11:52:22 <merni> in the rare case I have breakdowns enabled I just use depot orders :p
12:04:56 <xarick> found a bug with the servicing interval in the vehicle details window, when left arrow clicking and the value is 5%, it jumps to 90%
13:03:53 <xarick> placing buoys doesn't need to InvalidateWaterRegion
13:04:08 <xarick> the entire buy is traversable, isn't it?
13:05:26 *** gdown has quit IRC (Quit: Client limit exceeded: 20000)
13:07:34 <xarick> there are many more commands that can delete water tracks...
13:21:47 <xarick> i had a very old patch, now deleted, that would be useful here for the water region ivalidation
13:22:13 <xarick> it would detect any occurrence of water tracks being deleted
13:24:48 <kuhnovic> xarick: I vaguely remember there being an issue with buoys not being detected, and that's why I added that invalidation code. But tbh I'm not sure if it's still relevant. Doesn't really hurt though.
13:29:52 <_jgr_> I've been looking through it and haven't found any missing invalidations
13:30:17 <_jgr_> I've been adding some more automatic checks checks to my branch and will be running tests
13:31:05 <_jgr_> If I find anything I'll raise or PR it
13:33:09 <_jgr_> That said, I'm not really convinced about the save/load side
13:33:34 <_jgr_> As far as I can see, the data that is saved/loaded is not necessary at all
13:34:21 <_jgr_> It's not necessary to pre-init all regions after map generation either
13:36:54 <xarick> it could be repurposed to Check-if-it-needs-water-region-invalidation
13:37:30 <xarick> but 6 years of changes could mean there may be more places where invalidation is required
13:38:54 <kuhnovic> _jgr_: This is true. My reasoning behind it was that you'd rather have some time spent precalculating region instead of having to do it the first time you build a ship and let it travel a large distance.
13:40:14 <kuhnovic> _jgr_: This had something to do with network with desync, although it's been so long now that I forgot the details. Have to dig deep into my memory here, I've worked on this region pathfinder for about a year now 😛
13:41:12 <_jgr_> I don't see that it really helps with preventing desyncs, at least in it's current form
13:41:28 <_jgr_> You'd have to send the whole cache state for that
13:43:19 <_jgr_> I suppose we'll find out soon enough 😛
13:44:15 <_jgr_> Minor quibbles aside, it is a very nice piece of work
13:48:38 <peter1138[d]> Wasn't the saveload something to do with making sure the same region IDs are used after loading?
13:52:21 <kuhnovic> That shouldn't matter. Determining the region id should be deterministic, and even different IDs would be assigned then it wouldn't matter. These IDs are just to differentiate between different patches of water. The actual value of the ID doesn't have any effect on iteration order.
13:58:39 <_glx_> hmm could depot knows their water patch id ?
13:58:54 <peter1138[d]> With #11721, I tested static vs non-static and it's a non-issue 🙂
14:00:57 <xarick> the ultimate test: 15 AIs, all doing ships, it's time
14:01:11 <xarick> real AIs, not test samples
14:01:40 <_glx_> the placing too much buoys AIs ?
14:01:58 <xarick> well they all use buoys
14:02:05 <xarick> but i'll update mine to stop using them
14:02:25 <_glx_> they are annoying because they usually don't reuse buoys
14:03:21 <peter1138[d]> Hmm, station cargo filter only cares about waiting cargo. I wonder if it should know about both waiting and delivered, or if that should be a separate filter.
14:03:27 <kuhnovic> _jgr_: I think I remember why I introduced the savegame data. I traced it back to this comment: https://github.com/OpenTTD/OpenTTD/pull/10543#issuecomment-1458711185 . If there is any bug in the region invalidation logic (and there were quite a few), then a region might not get invalidated if a particular even happens, e.g. terraforming. If a new player would join that game then that particular
14:03:27 <kuhnovic> region would not be in sync with the other players, potentially causing a desync.
14:04:48 <xarick> okay terraforming ais that do ships... there's the triple A Hog
14:05:09 <xarick> nocab does little terraforming but it amasses ships
14:05:17 <xarick> shipai also good at amassing ships
14:05:35 <xarick> unsure about the notperfectai
14:05:39 <_jgr_> The data that is saved does not actually work around a region that is not invalidated
14:06:47 <_jgr_> If the region was marked valid and an invalidation is missed, it'll still be marked valid, but will be incorrect
14:07:58 <_jgr_> If a client joins after that, whether or not it immediately updates the region, it won't match the incorrect value on the server
14:08:31 <_jgr_> This type of cache mismatch is the most common source of desyncs
14:09:17 <_glx_> and they also could trigger a long time after the mismatch happened
14:10:43 <_jgr_> For some cases like vehicles it's pragmatic to send the possibly incorrect caches to clients to get around this, because of NewGRF misbehaviour
14:12:07 <_jgr_> That shouldn't be necessary for water regions though
14:16:31 <kuhnovic> I recently had another idea to make the region invalidation more robust: just invalidate every time a tile within the region is cleared
14:17:29 <kuhnovic> It's relatively easy to catch and it would do away with any of the potentially buggy invalidation logic that has to be implemented all over the place now. Then I'd say we would be able to ditch the savegame stuff.
14:18:39 <peter1138[d]> Might be expensive on 4kx4k map? Or just invalidate the region?
14:21:32 <xarick> Kuhnovic! take some ideas from the defunct #6935
14:21:43 <kuhnovic> Invalidation itself is just setting a boolean, so that shouldn't matter much. But any clearing done on non-water tiles would also result in the region getting invalidated. And if the pathfinder then stumbled upon that region, it will call the labeling procedure. So it will have some effect on performance.
14:23:18 <kuhnovic> xarick: A bit more specific please 😛 ?
14:24:34 <xarick> in 6935 I went through the places that "just invalidate every time a tile within the region is cleared", the situation is similar
14:25:47 <xarick> the purpose of that PR is different, but it has the similar situation as the need to check for invalidation
14:30:41 <kuhnovic> Your solution is actually similar to what I have now: multiple places where you need to call the same check. If you forget one then that particular functionality will not work as desired, in your case build over a ship track. In my case it will not update a region and potentially cause a desync 😛
14:32:28 <kuhnovic> The alternative is putting it in DoClearSquare (in landscape.cpp), which is called for any action that changes the landscape
14:33:09 <xarick> I think there's a few ones that skip clearing entirely, I think Object tiles
14:40:14 <_glx_> using Clamp<int> seems to do the rick
14:40:20 <_jgr_> The one already in MakeWaterKeepingClass covers a lot of the cases
14:40:45 <_jgr_> That said I think I've found one missing case now, cearing half-tile rails with water
14:42:08 <kuhnovic> That could very well be the case. It's hard to foresee all the edge cases 😦
14:43:26 <kuhnovic> I'm going to put my "just always invalidate" into a PR so we can give it a try. I'll also remove the savegame data. Then we can give it a proper test.
14:43:56 <xarick> I'm running a server btw
14:44:33 <xarick> if someone could join and get a desync
14:44:51 <xarick> then it means it's not working correctly
14:46:27 <kuhnovic> Ì'm pretty sure you will not get a desync even if there is a bug in the evaluation. That's because the invalidation state of the regions are in the savegame. Everything is copied to the new player "bug compatible".
14:47:52 <_jgr_> The invalidation state is just a boolean
14:48:20 <_jgr_> Even if the value is true at both ends, that does not by itself mean that the actual data is correct at both ends
14:49:09 <kuhnovic> The region labeling is based on the Tile data array, and that one is in the savegame. So yes the data should be in sync.
14:51:36 <_jgr_> The tile data array is mutable
14:54:43 <kuhnovic> _jgr_: What do mean with that exactly?
14:55:30 <_jgr_> The region labelling is a function of when the last update was made
14:56:39 <_jgr_> Going back to comments earlier about cache mismatches, you still can desync with this data in the savegame
14:57:50 <_jgr_> I can probably set up a test save to trigger the desync later, now that there's a known missing invalidation
14:58:00 <_jgr_> In the meantime I need to pop out for a bit
14:59:48 <xarick> I think I know what JGR means. a missing invalidate on an already "marked" region will leave the server in an invalid state, but the player joining in will see that it needs to do a force update, meaning that the joining player gets to "correct" the state, while the server didn't
15:00:12 <kuhnovic> You can uncomment some of the InvalidateWaterRegion calls to create additional broken cases. I still don't think it will cause a desync, but I will be happy (and a bit scared) if you can prove me wrong
15:00:59 <kuhnovic> xarick: You may have a point here... hmm
15:01:51 <_jgr_> I've spent a lot of time tracking down desync problems in that past
15:02:35 <_glx_> and that's why you check a lot more caches in JGRPP than we do in vanilla
15:04:09 <kuhnovic> _jgr_: I do not envy you. Those things are a real b*tch...
15:28:52 <kuhnovic> unsigned ints... it never gets old
15:39:21 <LordAro> it's more fun when you mess up int64 & int32 in the definition & declaration
15:40:05 <LordAro> second half of this integer? sure, that's a char*
15:43:17 <peter1138[d]> Remember when Minecraft had lots of signed-integer limits because Java doesn't have unsigned integers (iirc...)
15:51:05 <kuhnovic> KuhnovicviaGitHub: xarick _jgr_ The PR I talked about earlier
15:52:36 <peter1138[d]> You've left a TODO in there.
15:53:11 <xarick> beware of Object tiles
15:53:44 <xarick> sec, let me link to a related issue
16:00:19 <_glx_> hmm doesn't removing the chunk handler breaks loading savegames containing this chunk ?
16:03:14 <xarick> oh, you could check for water tracks before and after, and only do the invalidate call when there's been changes
16:05:08 <peter1138[d]> _glx_: It's probably recent enough to not worry.
16:08:34 <_jgr_> Presumably the savegame version needs to be unbumped as well in that case
16:09:19 <peter1138[d]> Don't think unbumping is wise 🙂
16:17:08 <_glx_> unbumping will just give a different error
16:18:11 <xarick> Broken savegame, unknown chunk type 🙂
16:18:30 <xarick> oh well, let's generate 64000 ship depots again
16:19:51 <peter1138[d]> If there is a nightly build with the chunk maybe we should handle it.
16:20:09 <_glx_> there are 2 nightly builds I think
16:20:17 <peter1138[d]> Last time we did a savegame rollback the nightlies were broken for a week so it didn't affect much 🙂
16:20:47 <_glx_> bad luck the nighlies were just fixed 🙂
16:23:31 <_glx_> a CH_READONLY should do it
16:25:46 <xarick> dbg: [map] Invalidated water region '(32,0)' what's the second number?
16:26:33 <xarick> strange, the Y coordinate is most of the time 0
16:29:49 <LordAro> i wouldn't be opposed to unbumping in this case
16:39:49 *** Wormnest has joined #openttd
16:44:31 *** gelignite has joined #openttd
16:47:09 *** HerzogDeXtEr has joined #openttd
16:47:57 <kuhnovic> peter1138[d]: Yes, on purpose. Makes it easier to test things 🙂
16:49:48 <kuhnovic> _glx_: Yes, but since that's a recent addition I hope to get away with it. It's not in any released version of openttd.
16:51:17 <kuhnovic> _jgr_: I did that initially, but I need that version bump to know when to initialize the map.
16:52:19 <kuhnovic> Now that I type that I realize that makes very little sense. Let me have a second look at it tonight, I think I know what I did wrong
16:53:28 <peter1138[d]> kuhnovic: I think nightlies count, as they are easy to get via Steam.
16:53:52 <peter1138[d]> Not sure what needs to happen to handle a chunk that we don't care about any more. Hopefully not much.
16:58:13 <_glx_> somethinh like ENGSChunkHandler
16:58:16 <peter1138[d]> Why did I decide I would go out on MTB...
16:58:22 <xarick> Invalidate water region happens very often on a 4k map
16:58:31 <xarick> possibly towns growing houses
16:59:07 <xarick> and there's a test and exec, I see it double
16:59:15 <xarick> should only need to call 1 time
17:01:56 <xarick> only a few cases where it doesn't duplicate
17:02:07 <_glx_> it's not because test/exec
17:02:20 <_glx_> the clearing is done only for exec 🙂
17:03:44 <_glx_> hmm could be AIs building docks
17:04:14 <xarick> there's only a GS and he's already done building 64k depots
17:09:52 <xarick> this->initialized = false;
17:10:00 <xarick> i thought it was gonna recompute
17:10:19 <xarick> it just switches a toggle
17:12:40 <_glx_> ok it's the town removing a house
17:13:38 <_glx_> and yes invalidating is "free"
17:13:44 <kuhnovic> Remember that invalidating a region is a light operation. It's not until the ship pathfinder enter that region that the region is actually updated.
17:14:00 <_glx_> the hard calculation is done when it's really used
17:16:10 <_glx_> I imagine the worst case is when the map was split in 2 different water bodies and they suddenly they are joined
17:17:00 <_glx_> and a ship wants to go on the other side
17:17:20 <xarick> we need 75000 ships instead, not 64k depots 🙂
17:18:07 <_glx_> the first ship will validate for all the others
17:18:30 <xarick> dont worry, I'll make towns grow very fast
17:19:04 <_glx_> oh and towns probably invalidate full land region
17:20:36 <xarick> gonna add a debug line somewhere when the force update is actually called
17:21:06 <_glx_> update is called when a pf run enters an invalidated region
17:21:37 <_glx_> so an invalidated full land region will never be updated
17:31:30 <kuhnovic> _glx_: This is actually not that bad. The only regions that need to be updated are the ones that were changed in order to make the connection between the two bodies of water. Everything else stays the same.
17:33:47 <kuhnovic> The worst case would be an AI continuously adding and removing objects all over the map so that all region get invalidated constantly. There's only one person I know that would make such an AI 😆
17:34:32 <_glx_> hehe it would be limited to 15 invalidations per ticks 🙂
17:35:02 <_jgr_> Not if you carefully place multi-tile objects to cross 4 regions at once 😛
17:36:05 <kuhnovic> And even then it would still be way faster than the trackdir-based pathfinder 🙂
17:39:46 <kuhnovic> Would it be an idea to invalidate the entire map and basically "start over" the moment a new player joins? That way all players would start fresh and there shouldn't be any desync issues. Feels a bit like a workaround, but I think it would work.
17:43:33 <xarick> `Debug(map, 0, "Force updated region '({},{})'", GetWaterRegionX(*this->tile_area.begin()), GetWaterRegionY(*this->tile_area.begin()));`
17:43:46 <xarick> wondering if that gets the right area
17:49:27 <xarick> they get all areas not initialized, i think it's not needed
17:50:56 <xarick> does the exact frame the player enters need it?
17:51:23 <xarick> maybe you're right, it's needed
17:54:46 <peter1138[d]> It is 100% chilly
17:55:45 <_glx_> a player joining starts with all regions invalidated, what matters is when they get updated the result is the same as other players
17:57:28 <xarick> excessive cpu valuator for AAAHog... you killed him
17:58:34 <_glx_> 100000 ops per valuator is quite high
18:01:33 <_glx_> ah no the limit is 500000
18:01:52 <peter1138[d]> weirdly griefing AI
18:06:23 <_glx_> actually it should now be possible to limit per call of the valuator/filter instead of the full valuate/filter run
18:06:50 <kuhnovic> _glx_: If all players have to update all regions then all their results will be the same. The region data is based only on the tile array and that one is synced.
18:07:43 <_glx_> I mean not invalidating others when a player joins should be fine if the update result is deterministic
18:08:01 *** bigyihsuan has joined #openttd
18:08:01 <bigyihsuan> peter1138[d]: i don't like looking at this
18:08:19 <kuhnovic> Problems arise when the first player has a region that should have been invalidated but isn't due to a bug. Now player two joins and the state of that region will be different.
18:09:01 <_glx_> yes but invalidating all on join is just a work around
18:11:14 <_glx_> because now all players will possibly get improperly marked regions during the run
18:11:50 <_glx_> without causing desync, but still broken
18:13:56 <peter1138[d]> Okay fixed that but...
18:15:08 <xarick> made a video showing how often it force updates
18:15:10 <peter1138[d]> Hmm, maybe I should only block it for AI.
18:16:14 <_glx_> xarick: it's not that much
18:16:17 <peter1138[d]> Seems a bit unequal though.
18:21:06 <xarick> shouldn't a GS be paused when a player is joining the server if the server is set to pause during join?
18:23:43 <xarick> i just joined, got that error
18:27:51 <xarick> Writing crash dump to disk...
18:27:51 <xarick> No method to create a crash.dmp available.
18:27:51 <xarick> Writing crash dump failed.
18:27:57 <kuhnovic> xarick: How many ships and what map size?
18:28:01 <_glx_> at least click on show report
18:28:30 <xarick> 4096x4096, 15 AIs, they just started this year, not many ships
18:28:53 <_glx_> about the crash dump failure, do you have breakpad installed ?
18:29:52 <xarick> "reason": "EXCEPTION_ACCESS_VIOLATION"
18:29:53 <_glx_> hmm but it should be installed automatically
18:30:24 <xarick> stacktrace is many lines with addresses, blank after
18:30:42 <_jgr_> Just post the JSON file as an attachment?
18:30:51 *** asymptotically2 has joined #openttd
18:30:53 <xarick> oh, right, this is discord
18:31:35 <kuhnovic> How can I place objects while not in the Scenario Editor?
18:32:07 <xarick> some newgrf lets you place water objects
18:34:08 <xarick> gonna try join with a debug build
18:35:55 <_jgr_> If this is the PR it's probably because the extra call to InitializeWaterRegions doesn't seem to be in it?
18:38:37 <xarick> looks like the vector is empty
18:39:11 <_glx_> Well I could have tell it was empty from the assert failure
18:39:21 <xarick> it has 256 regions on it
18:39:30 <xarick> but the loaded game is much larger
18:39:59 <_glx_> Regions from intro game ?
18:40:49 <xarick> yes, looks like that's it
18:41:35 <xarick> afterload needs to initialize the regions 🙂
18:42:12 <_jgr_> TBH, there's no reason to wait that long to allocate the array
18:42:16 <kuhnovic> You are testing my PR right?
18:42:35 <kuhnovic> I think I know what I did wrong with the loading. Will look at it soon.
18:42:36 <_jgr_> I've moved it to the end of AllocateMap in my branch, rather than having two separate paths for it
18:46:11 <kuhnovic> The build object command doesn't do proper clear tile checking. I guess this is why. It sure leads to some inconsistent behavior, such as being able to just overbuild an object without ever getting an error message.
18:47:27 <_jgr_> Objects have a flag for whether they can be overbuilt, most decorative objects will use it
18:48:29 <kuhnovic> Ah ok that makes sense then
18:49:10 <kuhnovic> But no clear command is ever issued in case of those water objects. I'll add a call to InvalidateWaterRegion() when the object is built.
18:50:01 <xarick> do objects have a flooding property?
18:50:17 <xarick> the ability to have water pass through them?
18:51:08 <xarick> not the right question but you get the idea
18:51:56 <_jgr_> Yes, see `GetFloodingBehaviour`
18:54:55 <kuhnovic> _jgr_: Where exactly did you put it? I can't seem to find AllocateMap anywhere
18:56:25 <_jgr_> Ah, it has a different name in vanilla
18:56:56 <_jgr_> You not want to put it directly in there, but in general there isn't any need to have separate map generation and load game paths
18:57:08 <truebrain> _glx_: wow, that friendlist of GOG is weird as ... well .. it is weird 😛
18:57:25 <truebrain> when you have the time, let me know if it mentioned I am in OpenTTD 🙂
18:58:28 <peter1138[d]> Buoy-less ships-only...
18:59:45 <truebrain> peter1138[d]: like ants!
19:01:35 <truebrain> peter1138[d]: your changes with `std::unique_ptr<NWidgetBase>` broke my PR 😦
19:01:41 <truebrain> I was keeping the pointer to the widget twice 😛
19:04:02 <truebrain> but luckily for me now, `->children` is now a sane object, so it compensates 🙂
19:40:42 <xarick> if (index >= static_cast<int>(_water_regions.size())) return; nice catch
19:41:25 <kuhnovic> Found out about that one the hard way 😛
19:41:50 <xarick> shouldn't that be an assert?
19:43:38 <kuhnovic> Maybe... I remember I added it because in certain in-between-games-situations a call would be made to InvalidateWaterRegion which was unexpected
19:44:30 <kuhnovic> Btw, if you want to open your previous savegame then you have to re-bump the version. I unbumped it in my latest changes
19:51:16 <xarick> InitializeWaterRegions(); I think this needs to be in afterload, gonna test if it causes desync
19:51:40 *** nielsm has quit IRC (Ping timeout: 480 seconds)
19:53:41 <kuhnovic> It's now done in Map::Allocate. But it only allocates the correct number of regions, it doesn't actually update them since at that stage the map is not actually filled with data.
19:59:47 <kuhnovic> This is what happened in your case. It would check if the savegame version was lower than X, if so then it would initialize the water regions. If version >= X then previously the water region state would be fetched from the savegame data block that was added. But that's no longer the case, so in your case it simply didn't create any water regions. And then things went wrong hehe.
20:04:28 <xarick> I'm very surprised I'm not getting a desync now, but I just started the game... will let the AIs fill it with ships
20:05:10 <truebrain> _glx_: figured out that via the website I can login and then go to my own profile and see my own status; so I managed to debug stuff myself, and it works \o/
20:05:36 <truebrain> there is a weird bug that when I close the game, it doesn't see it directly. But that is most likely because I run it in development build 🙂
20:13:32 <_glx_> yeah depending on where I look it says you are playing, or you are offline
20:15:55 <Rubidium> oh noes... you desync gog?
20:17:23 <truebrain> it is also fun when I start it after closing, as the first close times out, removing the "Playing" state 😛
20:17:31 <truebrain> despite I am playing again 🙂
20:17:35 <truebrain> but that is all a GOG issue; not a me issue
20:17:52 <truebrain> so as far I am concerned, this works (sufficiently)
20:20:22 <xarick> gonna try force update on afterload
20:20:29 <andythenorth> peter1138[d]: roads joining cities though... 😛
20:20:35 <andythenorth> JGR has a thing for it
20:21:14 <kuhnovic> xarick: Did you do anything specific, or was the game just running?
20:21:30 <xarick> i joined the server, waited about 5 seconds, desync
20:22:10 <xarick> it's still 15 AIs amassing ships
20:22:18 *** HerzogDeXtEr has quit IRC (Read error: Connection reset by peer)
20:22:32 <andythenorth> Interestingly when I ask GPT to stick to 'facts not hallucinations', it gives deliberately more vague information. Like a consumer product lawyer.
20:23:07 <andythenorth> `"Iron Horse" was developed by various members of the OpenTTD community, including multiple contributors who designed the locomotives and rolling stock within the set.`
20:23:19 <andythenorth> whereas without that prompt, it makes up actual author names
20:38:15 <xarick> I'm not getting a desync...
20:39:55 <xarick> I added this, then called it from afterload
20:40:17 <xarick> seems to solve the desync... so far...
20:42:52 <kuhnovic> Hmm, I wonder why that helps. Either you update the regions when you load the game (when you join a network game) or the pathfinder needs those regions and then updates them lazily... both _should_ yield the same results
20:43:19 <_glx_> would mean an invalidated region is used without triggering update
20:43:31 <kuhnovic> Can you try leaving / joining a bunch of times?
20:44:27 <_glx_> or the invalid state is wrong
20:45:41 <xarick> the state is only allocated
20:46:32 <xarick> I imagine the initial frame isn't synced when loading, and needs to
20:49:08 <xarick> should be enough clients
20:50:04 <kuhnovic> What if you disable the initialize call you added, recompile, and try to join? Or is that a hassle?
20:50:35 <xarick> that's what I tried first
20:50:40 <truebrain> Rubidium: how often are you going to have comments on something you already reviewed? 😛
20:50:48 <truebrain> this is like the 4th round of review on the same code now 😄
20:53:58 <kuhnovic> Wait, hold on Xarick... are all clients running the same code or not? Otherwise desyncs are pretty much guaranteed...
20:55:12 <Rubidium> truebrain: it the super annoying thing where you reread something quite a few times, and then once you post (or the next day) you see all kinds of problems with it. I've got that especially with huge diffs :(
20:55:43 <truebrain> there is also the case of: sometimes things are just "good enough" 😛
20:56:28 <xarick> funny can't get a desync now this is really strange
20:56:58 <xarick> i was using different client than the server
20:58:28 <_glx_> they should use the exact same version
20:58:59 <xarick> i only added the afterload code, i thought i wouldn't need to restart the server
21:00:01 *** gelignite has quit IRC (Quit: Stay safe!)
21:01:51 <kuhnovic> Is your crazy shipbuilder AI available somewhere?
21:02:06 <xarick> it's several AIs from bananas
21:03:58 <kuhnovic> Any one you recommend in particular?
21:05:19 <xarick> triple A hog, but currently it's getting cpu valuator crashes
21:06:09 <xarick> make sure you set them to only build ships
21:07:18 <kuhnovic> Alright I'll give those a try
21:08:08 <xarick> actually, it's okay to have some that build other types, as they usually do terraforms
21:10:53 <xarick> why can't I host 2 servers?
21:11:10 <_glx_> you can, just use 2 different ports
21:11:32 <xarick> ports? I thought that was now automated
21:11:51 <_glx_> it's automated on join using invite code
21:12:06 <truebrain> let's hope Windows still likes my PR 😄
21:12:06 <_glx_> but to host that won't change
21:15:34 <_glx_> it can generate a crashlog
21:15:35 <xarick> there's something else really I find strange happening... When I join, the AIs are told to save, and yet the client is already in, loaded, and only after that I get the AIs complaining too long to save
21:16:15 <kuhnovic> Oh man, it was a mistake to run this in debug. Those AI are killing my CPU 😛
21:17:10 <xarick> i cant get the 3980 server listed, probably windows firewall?
21:17:20 <truebrain> Rubidium: smart idea, the has_error / error merge .. for some reason, I always forget that is what a `std::optional` does 🙂
21:18:27 <xarick> ah, got it, had to manually add the server
21:18:49 <xarick> the automated server join dude fail
21:21:22 <_glx_> yeah std::optional is a 2-in-1
21:26:26 <xarick> no desyncs... I don't understand
21:26:57 <_glx_> the joys of desyncs, they never happen when you are looking for them
21:27:23 <_jgr_> Doing stuff randomly is not the way to track down desync problems
21:28:26 <kuhnovic> And joining the game with different builds certainly isn't either
21:29:55 <xarick> server client client / server client client
21:30:22 <xarick> each set uses the same version
21:30:42 <xarick> the top doesn't have afterload code added, the bottom one has it
21:31:01 <xarick> but funnily enough, I am not triggering a desync 😐
21:34:07 <kuhnovic> Tried to reproduce it myself. Game with multiple AI's building ships like crazy, me starting multiple clients joining the game. No desyncs
21:38:45 <xarick> found a bug... I restarted server, only one client rejoined
21:49:00 <xarick> and doesn't need to save
21:52:13 <kuhnovic> I also got zero desyncs
21:52:35 <kuhnovic> That isn't to say there is no desync potential here, but it looks promising
21:55:08 <xarick> I was worried about the very first frame that's loaded, I had a situation about road vehicles having an off by 1 tick synchronization issue with their path cache
21:56:28 <xarick> convinced myself it would fix itself after it checked the cache, but it ended up having the buses going different paths
21:57:11 <xarick> the tick 0 needed to be synced too
21:59:30 <_jgr_> Was that in one of your branches, or master?
22:00:00 <xarick> one of my branches, it was a PR too, but glx found a different solution
22:00:40 <xarick> it went with his solution, it was better, as no afterload conversion was needed
22:12:03 <peter1138[d]> truebrain: Can always be changed to `std::shared_ptr`
22:14:05 *** keikoz has quit IRC (Ping timeout: 480 seconds)
22:16:46 <peter1138[d]> That was the reason I changed the NewGRF Widget from a Container to... not a container.
22:17:07 <peter1138[d]> The vector of children was not really convenient with how that one works.
22:21:21 <truebrain> I could read `this->children`, so I could iterate over the children. As you made that a nice vector now, it was also much more clear how to do that
22:21:25 <truebrain> with head/tail stuff I got a bit lost 😛
22:21:37 <truebrain> so there wasn't any need to also keep track of my widgets; the verticalthingy already did it
22:21:41 <truebrain> so yeah, nice refactor 😄
22:24:34 <xarick> some spikes from time to time
22:34:09 <xarick> looking at the code, i'm not sure the object handling is done right
22:37:17 <xarick> probably doing okay, there's a DoClearSquare in MakeWaterKeepingClass
22:37:30 <_glx_> I think the spikes are when AIs mass start their new ships
22:38:10 <xarick> there is an intermitent spam of vehicle vistiting stations for the first time, I don't think it's that
22:38:27 <peter1138[d]> They still leave depots one at a time right?
22:41:06 <_glx_> might also be the first pf run of a new line visiting new regions
22:43:20 <xarick> I would place InvalidateWaterRegion(t) inside BuildObject
22:43:37 <xarick> away from CmdBuildObject
22:43:51 <xarick> the area is iterated there
22:50:01 <kuhnovic> Why? What difference would that make?
22:50:40 <xarick> one less tile area iteration 😛
22:52:57 <kuhnovic> But you need to invalidate every tile that the object sits on. It might span multiple regions.
22:54:45 <xarick> would move it to line 130
23:02:19 <_glx_> yeah small optimisation
23:08:57 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
23:26:05 <peter1138[d]> Is a water region a square chunk?
23:28:50 <peter1138[d]> Just like Minecraft 🙂
continue to next day ⏵
