IRC logs for #openttd on OFTC at 2024-11-17
⏴ go to previous day
00:00:28 *** HerzogDeXtEr has quit IRC (Read error: Connection reset by peer)
00:08:15 <peter1138> If only someone would tell him about TicToc's limitations.
00:08:28 <_glx_> xarick: a function exist for that, I'll let you check documentation about std::tuple
00:09:09 <_glx_> (hint: it's a non-member function)
00:14:41 <_glx_> and it's used in the example
00:16:35 *** Wormnest has quit IRC (Ping timeout: 480 seconds)
00:26:49 <michi_cc[d]> You don't need the function anymore with C++17, a simple keyword is enough.
00:53:38 <_glx_> depends if the variables exist before the call or not
00:58:21 *** Wormnest has joined #openttd
02:20:00 *** Wormnest has quit IRC (Ping timeout: 480 seconds)
02:32:13 *** ChanServ sets mode: +v tokai
02:39:03 *** tokai|noir has quit IRC (Ping timeout: 480 seconds)
02:47:37 *** Wormnest has joined #openttd
03:16:02 *** godbed_ has joined #openttd
03:19:23 *** D-HUND has quit IRC (Ping timeout: 480 seconds)
03:19:33 *** debdog has quit IRC (Ping timeout: 480 seconds)
04:46:23 <DorpsGek> - Update: Translations from eints (by translators)
09:45:46 <johnfranklin> After removing FIRS in cargo label page, xussr author was very angry…
09:51:57 *** gelignite has joined #openttd
10:34:01 <johnfranklin> George is much older than 12.
10:37:29 <johnfranklin> Is this a joke on xussr authors?
10:40:31 <johnfranklin> Andy is traditionally britishly selfish. I can only say that.
10:46:55 *** HerzogDeXtEr has joined #openttd
10:48:39 <johnfranklin> But I don't use any translator...
10:53:23 <peter1138> Who's misinterpreting who here
10:54:38 <belajalilija> i understood andy
10:55:21 <belajalilija> hes just saying that the xussr people are *not* like the whiny shits you'd meet on reddit
10:56:39 <belajalilija> nothing he said was selfish
10:57:16 <belajalilija> i have my personal biases
12:09:36 <michi_cc[d]> Looks like an accidental duplication at first glance, until you really read it.
12:09:55 <michi_cc[d]> Maybe it should be one paragraph.
12:20:28 <michi_cc[d]> Yeah, that looks better.
12:27:59 <michi_cc[d]> You often recommend setting CC_PIECE_GOODS on cargoes as a fallback (e.g. for CC_GAS_BLUK). So if I'd make a vehicle set that does know about CC_GAS_BULK, I would assume I could use CC_GAS_BULK as an exclude on my normal piece goods wagon if I only want it in my specialized vehicles, which the guide says not to do.
12:30:40 <michi_cc[d]> CC_REFRIGERATED is something I would expect as an exclude, too. If I have refrigerated vehicles, why allow it in non-refrigerated vehicles?
12:31:05 <michi_cc[d]> andythenorth: I'm not questioning the fallback, but why CC_PIECE_GOODS is not allowed as an exclude.
12:31:46 <_glx_> It's a super generic class
12:33:10 <_glx_> Exclusion is fine if there's always one vehicle accepting excluded class
12:34:35 <_glx_> Class logic is simple, any of included, none of excluded
12:34:52 <michi_cc[d]> I know that the cargo classes specs were always quite vocal about not excluding the basic classes, but this never seemed useful to me.
12:36:38 <_glx_> Issue was covered without non_covered
12:37:29 <_glx_> Some class are fine without the opposite one
12:43:55 <michi_cc[d]> One might question if saying CC_REFRIGERATED is only food is good. One could for example also use it to distinguish cryogenic gases and then code a cryogenic tanker as include CC_REFRIGERATED and exclude CC_PIECE_GOODS|CC_LIQUID_BULK|CC_OPEN_BULK|CC_COVERED_BULK|CC_POWERED_BULK.
12:47:32 <michi_cc[d]> I think my point is actually more that the strict "only use as include"/"only use as exclude" rule (not invented by you of course) is unnecessarily limiting, as I can come up with various examples where inverting it produces useful results.
12:53:47 <_glx_> I mean use 0xFFFF for allowed classes, which is a bitmask of all classes
12:56:14 <_glx_> You'd need infinite amount of bits for super precise classes, as there will always be some very specific cases
12:57:53 <_glx_> Plastic stuff can come from refinery
12:58:24 <michi_cc[d]> Hmm, would there by any point in having a vehicle cargo class refit property that really does a proper AND? I.e. only mark cargoes that have all the classes set as refittable?
13:00:02 <_glx_> With AND you'd only need one bit for potable or not
13:00:36 <_glx_> Usable as AND and NOT AND
13:02:13 <_glx_> My logic failed on first tey
13:04:14 <_glx_> Main issue with AND, OR and NOT AND is it's easy to make cargo not accepted at all
13:06:46 <_glx_> And relying on cargo authors setting the right classes
13:07:55 *** ChanServ sets mode: +v DorpsGek
13:07:55 *** ChanServ sets mode: +v Terkhen
13:07:55 *** ChanServ sets mode: +v planetmaker
13:07:55 *** ChanServ sets mode: +o orudge
13:07:55 *** ChanServ sets mode: +v blathijs
13:08:09 *** Emru has quit IRC (Quit: Emru)
13:08:50 <michi_cc[d]> Hmm, code has `cargo_allowed` to store the OR mask and `cargo_disallowed` for the AND NOT mask. How to call a AND mask? `cargo_required`, `cargo_allowed_and`, ...? Any better idea?
13:11:31 <michi_cc[d]> Set theory calls this intersection, but nobody will get that in variable name I guess.
13:14:07 *** gelignite has quit IRC (Quit: Stay safe!)
13:19:08 <xarick> `std::tie(closest_dist, ClampTo<uint8_t>(count)) = CountIndustryMatchingTypeAndLayout(ind_index, layout_filter, town_filter, current);`
13:19:08 <xarick> std::tie is complaining about the clamp
13:20:13 <_jgr_> That doesn't make sense, if you want to clamp it, do it after the assignment
13:20:46 <xarick> original code doesn't even clamp
13:20:57 <xarick> it's potentially buggy
13:23:01 <_glx_> std::tie can only assign, nothing prevents you to modify the values after
13:33:06 *** nielsm has quit IRC (Ping timeout: 480 seconds)
13:33:35 <michi_cc[d]> andythenorth: Would you want to know the cargo label or the translated cargo ID for your custom_refittable_rule_cb?
13:34:56 <_glx_> don't forget cargo classes are handled on newgrf load and converted to cargoID lists IIRC
13:36:15 <_glx_> same for label based refits
13:47:01 <michi_cc[d]> Yes, but I don't want a real dynamic CB anyway, just one that is run once after NewGRF laod.
13:47:26 <xarick> does the std::for_each accepts "break"?
13:53:09 <michi_cc[d]> Eh, it's fine if we assume cargo classes stay 16-bit :
14:03:38 <peter1138> Cargo class translation table, 64 bits internally.
14:06:23 <michi_cc[d]> Is `_gted[e->index].defaultcargo_grf` always more important than `e->GetGRF()`?
14:08:27 <peter1138> I'm not sure where all that lot came from, but I guess it's because the grf that installed the act3 matters for translation in the act2s
14:10:43 <michi_cc[d]> Hmm, I really need a better prop name. The old OR prop is " Cargo classes allowed". So how to call a AND prop? " Cargo classes allowed AND" just doesn't really cut it. "Cargo classes allowed intersection" would be the set theory name, but meh...
14:11:33 <michi_cc[d]> andythenorth: If every cargo gets its own cargo class, you now have to cargo labels to choose from 😛
14:13:07 <michi_cc[d]> Union is "is part of at least one of the sets", while intersecion is "is part of all sets".
14:13:18 <michi_cc[d]> I.e. union == OR, intersection == AND
14:14:50 <michi_cc[d]> And AND NOT is more or less the set difference
14:17:11 *** godbed is now known as debdog
14:40:36 <xarick> `if (Industry::HasMatchingIndustryTypeCondition(conflicting, is_conflicting)) {`
14:40:36 <xarick> my variable names and function names are sad
14:44:36 *** gelignite has joined #openttd
14:45:56 <michi_cc[d]> Hmm, will andy ever re-apear on the IRC bridge? The conversation looks a bit strange over there 🙂
14:49:29 <_glx_> main question is how would AND work when filling the refit mask
14:50:42 <michi_cc[d]> Any cargo that has all the classes set that are mentioned in the new prop is included. Note that it is not an exact match, so the cargo can define more classes.
14:51:26 <michi_cc[d]> Exclude and individual cargo in/exlude are then processed as usual.
14:53:12 *** mindlesstux has quit IRC (Quit: Ping timeout (120 seconds))
14:54:44 <_glx_> didn't see there was a PR
14:55:06 *** mindlesstux has joined #openttd
14:55:30 <michi_cc[d]> Prop 28 (for trains) is `allowed = (cargo classes AND prop28) != 0`, while the new prop is allowed = (cargo classes AND prop31) == prop31`.
14:56:34 <michi_cc[d]> andythenorth: Yes, you were spam-killed after writing an unicode character similar to 2 / 3 that I don't want write properly here 🙂
14:57:41 <_glx_> I'd rename allowed_and to required
14:58:33 <michi_cc[d]> Hmm, maybe `cargo_required_classes` to be more clear?
14:59:39 <michi_cc[d]> The thing with required is that it doesn't replace the `cargo_allowed`, just adds a different source for allowed.
15:00:39 <michi_cc[d]> Going from set operations, it should be `cargo_allowed_union` and `cargo_allowed_intersection`, but that won't tell you anything if you don't know set maths.
15:03:15 <_glx_> also using union and intersection at the same time may lead to unexpected results
15:03:33 <michi_cc[d]> Actually, yes, I think I'll use `cargo_allow_any` and `cargo_allow_all`.
15:05:52 <_glx_> any and all are mutually exclusive
15:06:47 <_glx_> in the proposed implementation
15:07:30 <michi_cc[d]> _glx_: Not really, only if you use the same classes in both.
15:07:50 *** alexlehm has joined #openttd
15:08:27 <michi_cc[d]> Like having CC_MAIL|CC_EXPRESS in any and CC_BULK|CC_REFRIGERATED in all is fully okay.
15:09:11 <_glx_> I think the main usage would be CC_MAIL|CC_EXPRESS, both with CC_COVERED
15:09:39 <michi_cc[d]> Eh, maybe change the comments as well. But what is one more PR push 🙂
15:13:48 <michi_cc[d]> Maybe in the usual NewGRF author <-> developer canyon, where NewGRF authors develop complicated work-arounds instead of just asking for a proper built-in solution 🙂
15:17:38 <_glx_> what is currently possible is CC_BULK AND NOT CC_COVERED, what is not possible is (CC_MAIL OR CC_EXPRESS) AND CC_COVERED, and I don't think the PR changes that
15:18:57 <michi_cc[d]> _glx_: It has a new callback, so "anything" is possible now 🙂
15:21:49 <_glx_> if there's a callback then adding a new prop seems unneeded
15:22:15 <_glx_> it just increases the foot gun size
15:22:34 <michi_cc[d]> Callback is more complicated, but it is indeed two independant changes in one PR (but then that's why it's a draft PR).
15:24:17 <_glx_> so props are (xxx OR yyy) AND NOT (...) or (xxx AND yyy) AND NOT (...) and callback if you want to mix OR and AND
15:24:33 <michi_cc[d]> One could also add a new prop with a slightly different meaning instead, e.g. (using train prop numbers), including any cargo where `(classes AND prop28) != 0 && (classes AND prop31) == prop31)`
15:25:48 <_glx_> that looks like the most demanded feature
15:25:50 <michi_cc[d]> _glx_: Mixing the old prop28 (OR) and the new prop31 (AND) can be useful, but probably only in special cases.
15:27:13 <_glx_> you already have something and not
15:27:16 <michi_cc[d]> `[something] AND food` is the new prop31, and the other the old prop28/prop29 combo.
15:27:50 <brickblock19280> andythenorth: I think it does
15:28:18 <_glx_> but your prop31 implementation means all something and FOOD I think
15:28:37 <_glx_> while authors usually want any something and FOOD
15:28:38 <michi_cc[d]> Sure, but most of the time the `something` will just be one class.
15:29:19 <michi_cc[d]> E.g. for a food tanker you want CC_LIQUID AND CC_FOOD, and not CC_LIQUID OR CC_BUILK etc.
15:30:58 <michi_cc[d]> Though in that sense re-defining the new prop to really mean `required` and always combined with prop28 does make sense, because it also covers the simple A AND B case.
15:32:13 <michi_cc[d]> So, prop28 -> list of cargo type candidates -> filtered by new prop31 -> final includes type list.
15:33:43 <_glx_> and will seem more logical from a GRF author point of view I think
15:34:13 <michi_cc[d]> Okay, so let's revert some of the changes for a new PR revision that will still be untested 🙂
15:36:12 <michi_cc[d]> I will make a simple test GRF if there's actually interest in merging this, but not before.
15:36:41 <_glx_> I know andy had some ideas requiring AND
15:37:02 <_glx_> using covered and non covered hoppers
15:39:45 <_glx_> well with the new proposition you can define a covered hopper as CC_BULK and CC_COVERED, and a non covered one as CC_BULK and not CC_COVERED
15:42:04 <_glx_> so a cargo with just CC_BULK will only go in non covered
15:42:30 <_glx_> and a cargo with CC_BULK|CC_COVERED will only go in covered one
15:45:19 <_glx_> but if vehicle set only has one hopper type it will just set it to CC_BULK and accept both covered and not covered (as intended by classes)
15:45:24 <peter1138> I look away for 5 minutes (hours) and there's all sorts of new features going on
15:46:47 <michi_cc[d]> Okay, new version of the prop, PR description will updated in a second.
15:48:42 <peter1138> I should read it be commenting, right?
15:49:36 <peter1138> Hmm, what's the purpose to adding a new property and a callback?
15:49:59 <michi_cc[d]> "In detail, the existing prop 28/1D/18/18 select a candidate list of refittable cargos that have at least one of the cargo classes from the prop set. The new prop then filters this list by only allowing cargos that have all of the cargo classes from the new prop set."
15:50:26 <michi_cc[d]> peter1138: To give options for trashing on just half the PR 🤣
15:50:41 <michi_cc[d]> train/road/ship/aircraft
15:51:17 <peter1138> So cargo_allowed_required is a bit weird.
15:51:25 <_glx_> the annoyance of similar props with different numbers
15:51:32 <michi_cc[d]> Basically the callback is the universal solution, but it is also more complicated to use than a static prop.
15:51:32 <peter1138> "cargo_required" makes more sense to my brain.
15:51:50 <michi_cc[d]> But it is only evaluated together with cargo_allowed.
15:51:53 <peter1138> "cargo_allowed" = any of these, "cargo_required" = all of these
15:52:18 <michi_cc[d]> That was basically version 1 of the prop, but we are now at version 2 🙂
15:52:28 <peter1138> Who dissented from that? 😒
15:52:53 <michi_cc[d]> Version 2 can do the same as version 1, but more.
15:52:56 <_glx_> it's any of these classes also having these classes
15:53:28 <michi_cc[d]> 0, 1, 2 for 0 -> leave it like the prop, 1 -> allow, 2 -> disallow.
15:53:36 <michi_cc[d]> failed callback is treated as 0
15:54:01 <peter1138> The behaviour of the property looks fine to me, I just think the naming is weird.
15:54:04 <_glx_> any of allowed also having required, but not disallowed
15:55:47 <michi_cc[d]> I kept the `allowed` in the name because it is combined with `cargo_allowed` and doesn't do anything on its own.
15:56:11 <michi_cc[d]> I.e. if `cargo_allowed` is 0/unset, `cargo_allowed_required` is simply ignored.
15:58:58 <michi_cc[d]> andythenorth: Would you expect to test for cargo labels that you don't also have in your cargo translation table? I'd prefer to get rid of the label because it fully blocks one of the two normal callback params, leaving less room for future ideas.
16:00:35 <michi_cc[d]> andythenorth: That is because most of the time the GRF doesn't actually include the label, but the local ID.
16:00:51 <_glx_> labels should be handled by 2C/2D (train)
16:00:54 <michi_cc[d]> So NFO wouldn't be different.
16:01:21 <michi_cc[d]> _glx_: So an argument for not passing the label, right?
16:01:55 <_glx_> yes, it's already easy to handle with props, no need in callback
16:03:35 <michi_cc[d]> Well, I'd still leave the local ID in one param, just in case some wants to only do the callback.
16:04:37 <_glx_> though 2C/2D works with index to cargo table, not directly labels
16:04:40 <michi_cc[d]> It could also later be used as a param for a new global cargo 60+x variable to query further cargo info (like badges? 😛 )
16:07:11 *** keikoz has quit IRC (Ping timeout: 480 seconds)
16:08:54 <michi_cc[d]> Okay, label is gone in the callback
16:13:46 <michi_cc[d]> Still not sure though if using `_gted[e->index].defaultcargo_grf` over `e->GetGRF()`for the callback is actually the right thing.
16:17:03 <michi_cc[d]> Indeed, You'd only need NON_POTABLE if you actualy want 3 cargo "qualities".
16:20:28 <johnfranklin> I think bit 13 14 can be used as
16:20:35 <johnfranklin> "potable, nonpotable, neither"
16:20:50 <johnfranklin> And there is bad news
16:21:04 <johnfranklin> Temporal8 used cargo bit 15 to code his "real trains"
16:22:34 <peter1138> Not treating potable/non-potable as binary (it's either one or the other) was the reason for two bits.
16:23:01 <peter1138> Most of Temporal8's stuff just doesn't work well.
16:23:32 <_glx_> two bits were necessary because only AND NOT exists
16:25:27 <peter1138> Two bits were necessary because ALWAYS being one or the other isn't necessary.
16:26:03 <_glx_> with 1 bit it's potable or don't care
16:26:06 <peter1138> (andy's claim in frax that cargo types should set one or the other isn't quite correct, imho)
16:33:18 <johnfranklin> I reminded temporal8
16:38:33 <peter1138> He released TGV looking like this.
16:38:41 <peter1138> It's quite obviously wrong.
16:39:11 <peter1138> So I wouldn't be too worried about anything of his not working properly :p
16:39:59 <_glx_> and I don't want to imagine how they handle rail curves
16:41:36 <johnfranklin> He indeed caters many players
16:41:43 *** benjaminv has quit IRC (Ping timeout: 480 seconds)
16:41:43 <peter1138> No need to imagine 🙂
16:41:56 <johnfranklin> Belgi, Argus, etc, they more on forums
16:42:40 <_glx_> beautiful sprites, but don't fit in game
16:44:51 <peter1138> These trains also don't need electrified rail 🙂
16:45:55 <peter1138> Real Industries's cargo types don't use bit 15, so I dunno what that's about.
16:47:06 <peter1138> Two PASS cargo types. I think that's probably not going to work and should be illegal?
16:48:06 <peter1138> I guess we don't ever actually check to see if a label is already in use.
16:48:15 <johnfranklin> tbh temporal8 treat such things as "studio game workshops"
16:48:49 <peter1138> Should setting a label replace the previous use of it?
16:52:11 <peter1138> BuildCargoLabelMap does at least check for dupes.
16:52:31 <peter1138> But prop 17 doesn't check.
16:53:40 <johnfranklin> Why are all Japanese camera brands so expensive? I remember my family buying a basic model from Fujifilm in 2012 and not that expensive.
16:54:30 <peter1138> What happened to CC_NON_POTABLE?
16:54:47 <johnfranklin> Was CC_NON_POTABLE for lunch?
17:34:43 *** Flygon has quit IRC (Read error: Connection reset by peer)
17:56:12 <xarick> i have a weird massive sudden overhead
17:57:30 <xarick> when i pass a function from a function to another as a parameter, does this incur in some huge overhead?
18:02:48 *** Wormnest has joined #openttd
18:11:06 *** benjaminv has joined #openttd
18:14:12 <xarick> how do i pass reference functions or something
18:15:28 <kuhnovic> I'm using Wentbourne for some performance comparison. It's a great savegame, but I'm afraid it's no longer representative for ships.
18:15:33 <kuhnovic> Perhaps I should remove all the buoys.
18:20:36 <xarick> seems that I solved the massive overhead
18:21:25 <xarick> what a giant difference
18:22:31 <xarick> oh, kdtree.. that's the wrong test
18:27:39 <peter1138> 70,000 ships isn't particularly useful for anything.
18:29:29 <kuhnovic> I want a savegame that somewhat represents (heavy) gameplay. Wentbourne is that, except that nobody would build use buoys nowadays. As a results all ship PF runs are super short, and they don't represent real world use any longer.
18:30:13 <xarick> turn it into a 5000 ship save
18:30:28 <peter1138> Well, you can try removing the buoys?
18:30:48 <peter1138> Removing all the orders will be painful.
18:31:25 <peter1138> Hack saveload to not load buoys...
18:32:20 <_glx_> or skip buoys when processing orders
18:33:15 <kuhnovic> Buoys seem to persist even if you delete them. I'll probably have to hack some code somewhere to remove them from all orders.
18:33:20 <peter1138> I think it's more useful to produce a savegame without them (and the orders) so that a normal version can load it later without.
18:33:58 <_glx_> load savegame in editor and insert a GS to remove buoys and orders
18:34:36 <_glx_> starting with orders, because I don't think you can remove a used buoy
18:34:39 <xarick> _glx_: i did that when I wanted to remove ship depots from the orders
18:35:47 <xarick> who's a overhead expert? 🙂
18:36:03 <kuhnovic> You run 70K ship savegames, so I guess you are 😛
18:36:27 <peter1138> Oh, didn't I patch #309 so that it didn't use advanced action1 if it wasn't necessary?
18:36:51 <peter1138> (Or at least tried to)
18:37:38 <_glx_> and I didn't steal it 😉
18:42:15 <xarick> I'm trying to approach Peter's CheckIfFarEnoughFromConflictingIndustry and GetClosestIndustry results, but my overheads hurt. These two functions are basically iterating over the same number of industries
18:43:10 <kuhnovic> Man, this small YAPF change I made looks very promising. These are the preliminary results of my performance comparison:
18:43:10 <kuhnovic> [2024-11-17 19:36:41] dbg: [yapf:0] t : n = 768, skipped = 18 (2.3%), total ratio = 0.9776 (2.24% faster)
18:43:10 <kuhnovic> [2024-11-17 19:36:41] dbg: [yapf:0] r : n = 2936, skipped = 46 (1.6%), total ratio = 0.8097 (19.03% faster)
18:43:10 <kuhnovic> [2024-11-17 19:36:42] dbg: [yapf:0] w : n = 1985, skipped = 40 (2.0%), total ratio = 0.9530 (4.70% faster)
18:43:11 <kuhnovic> [2024-11-17 19:36:42] dbg: [yapf:0] ^ : n = 1265, skipped = 19 (1.5%), total ratio = 0.9977 (0.23% faster)`
18:43:11 <kuhnovic> And with a ship-only 128x128 save without buoys, slightly more representative of the current way people use ships:
18:43:13 <kuhnovic> [2024-11-17 19:38:05] dbg: [yapf:0] w : n = 1193, skipped = 13 (1.1%), total ratio = 0.9197 (8.03% faster)
18:43:15 <kuhnovic> [2024-11-17 19:38:05] dbg: [yapf:0] ^ : n = 296, skipped = 0 (0.0%), total ratio = 0.9828 (1.72% faster)`
18:47:44 <kuhnovic> I changed AddNewNode to first check if a node is closed. If it is it jumps out of the function immediately, which prevents potentially expensive cost calculations. t = train, r = road, w = water, ^ = water regions
18:50:08 <peter1138> Not sure how it translates to game performance though.
18:51:17 <xarick> 0% improvements on 70k game=
18:51:45 <kuhnovic> Depends on the amount of RVs and how long their routes are. But in general it's worth squeezing a bit of extra performance out of the PF since it is rather heavy.
18:52:26 <_glx_> RVs on grid pattern can be costly yes
18:53:38 <_glx_> and worse if the station is actually not reachable
18:54:58 <kuhnovic> Unreachable destination in general are the absolute worst case for an A* pathfinder
18:55:04 <_glx_> they used to redo the run every tick
18:55:47 <_glx_> now it's only when they change tile
18:56:03 <kuhnovic> Make much more sense. And there's caching too.
18:56:27 <_glx_> caching for unreachable doesn't work very well
18:57:48 <kuhnovic> Nope that's true. That's why the ship PF generates a random path for the ship. Keeps it busy for a while until it does the next call. It also makes it appear properly lost.
18:58:01 <kuhnovic> xarick: `Xarick 70K ships
18:58:01 <kuhnovic> [2024-11-17 19:56:20] dbg: [yapf:0] w : n = 2914, skipped = 227 (7.8%), total ratio = 0.9281 (7.19% faster)
18:58:01 <kuhnovic> [2024-11-17 19:56:20] dbg: [yapf:0] ^ : n = 1528, skipped = 133 (8.7%), total ratio = 0.9676 (3.24% faster)`
18:59:31 <xarick> I wonder if paths become worse though
19:00:29 <kuhnovic> Nope, they are identical
19:03:57 <johnfranklin> I bought a Kodak PixPro WPZ2
19:05:55 <peter1138> Yeah, RVs do cache but not when they are near the destination.
19:06:04 <peter1138> But hopefully if they are near it's not that costly.
19:07:09 <kuhnovic> Those should be relatively quick. Low amount of nodes and typically just a simple piece of road ahead with not too many ways to branch off.
19:08:24 <peter1138> Hmm, how do I fiddle with company allow lists?
19:09:15 <_jgr_> authorized_key console command probably easiest
19:15:10 <johnfranklin> Turkey (food) tastes very bad
19:16:03 <peter1138> So I'm slightly confused as to why passing a std::string to SlObject works, when the description is for a struct whose first element is std::string.
19:16:25 <peter1138> I mean it does work, but feels a bit weird.
19:19:27 <_jgr_> If the string is the first member of the struct, then the string and the struct have the same address
19:22:57 <johnfranklin> is there any way to forbid players building statues?
19:26:07 <peter1138> Sure. It feels a bit "undefined behavioury" 🙂
19:37:43 *** SigHunter has joined #openttd
20:04:24 *** Tirili has quit IRC (Remote host closed the connection)
20:07:29 <michi_cc[d]> But it is called for each defined cargo
20:07:44 <michi_cc[d]> So SELF is the vehicle/engine scope.
20:07:57 <michi_cc[d]> But basically in purchase list context.
20:11:39 <michi_cc[d]> Even worse, my CB is only run once directly after NewGRF loading. So don't do any date dependent stuff, or you have a nice desync.
20:12:30 <michi_cc[d]> Okay, looks like I managed to cobble a test-GRF together and the code apparently actually works 🙂
20:12:54 *** Wormnest has quit IRC (Quit: Leaving)
20:22:09 <michi_cc[d]> Test NewGRF uploaded.
20:22:48 <michi_cc[d]> Needs to be used in *sub-tropical* climate, as this is the only one that has cargos with multiple classes in the default game (toyland doesn't count 🙂 )
21:05:53 *** gelignite has quit IRC (Quit: Stay safe!)
21:07:11 <peter1138> var 0x10 is classes, var 0x18 is translation table slot.
21:08:32 <peter1138> GetVehicleCallback(callback, param1, param2, engine, v)
21:08:47 <peter1138> The patch passes classes as param1, and slot as param2.
21:09:06 <peter1138> So you need the lower 16 bits of var 0x10.
21:10:49 <peter1138> michi_cc[d]: Run it every time cargo refittability is needed... that'll perform well and not have any unexpected problems...
21:11:07 <peter1138> Do it in NFO. Everything makes sense in NFO.
21:14:13 <michi_cc[d]> andythenorth: My test GRF ha the equivalent of
21:14:13 <michi_cc[d]> switch (FEAT_TRAINS, SELF, sw_refit, extra_callback_info1 & bitmask(CC_BULK, CC_REFRIGERATED)) {
21:14:13 <michi_cc[d]> bitmask(CC_BULK, CC_REFRIGERATED): return 1;
21:15:35 <michi_cc[d]> Strictly speaking I should have ANDed extra_callback_info1 with 0xFFFF first, has the specs will say the other bits are reserved/undefined.
21:16:28 <michi_cc[d]> Scratch that actually, for the test I did, this is not needed. For directly switching on extra_callback_info1 OTOH it is needed.
21:17:12 <michi_cc[d]> extra_callback_info1 and extra_callback_info2 should always be available, but I don't know if NML has convenience shortcuts for certain callbacks.
21:18:19 <michi_cc[d]> Unless you explicitly return 2 for all unwanted cargos, the normal refit mask rules will apply.
21:18:45 <peter1138> Is this in vanilla or with the PR? 🙂
21:19:09 <peter1138> Have we reintroduced everything with FIRS being Acid...
21:26:14 <xarick> Copilot suggested me to change my weird structure into unordered_multimap
21:28:27 <peter1138> Do you need it ordered or not?
21:29:31 <xarick> it would be nice if it were ordered, but it that takes a hit on performance, I can go without it
21:29:34 <_glx_> do you really need a multimap ?
21:30:49 <peter1138> I could PR my industry type lists...
21:31:10 <xarick> It suggested going from this:
21:31:10 <xarick> ```static std::array<std::vector<std::pair<TownID, std::vector<IndustryID>>>, NUM_INDUSTRYTYPES> counts;```
21:31:10 <xarick> ```static std::array<std::unordered_multimap<TownID, IndustryID>, NUM_INDUSTRYTYPES> counts_map;```
21:31:49 <_glx_> both are ugly to be honest
21:32:27 <_glx_> but multimap looks better
21:42:53 <peter1138> Waiting for it to finish compiling again before I run some benchmarks.
21:43:25 <peter1138> It's more of a refit test, isn't it?
21:44:01 <peter1138> It updates the mask internally, but it doesn't provide or receive a mask.
21:44:23 <peter1138> Does it need a CBM slot?
21:44:51 <peter1138> Like, if I have 10,000 vehicles loaded, and each one is testing up to 64 cargo types, that could take a while.
21:46:19 <peter1138> And 10,000 is not contrived.
21:46:29 <peter1138> Loading just Iron Horse uses 10,000 IDs.
21:46:40 <peter1138> I'm still not quite show how andy managed that.
21:47:36 <_glx_> yeah seems like the callback is always executed, could benefit being off unless vehicle says it has it
21:47:52 <peter1138> I suspect Iron Horse might use it for every vehicle :p
21:48:21 <peter1138> I like the required mask property. That's simple and cheap, and doesn't allow for desyncs.
21:48:48 <peter1138> 10,000 in one set makes me regret coming up with variants tbh.
21:48:57 <peter1138> I had envisaged about a 10th of that.
21:52:53 <michi_cc[d]> The PR could use some comments, just saying 🙂
21:53:23 <michi_cc[d]> Giving it a callbask mask is certainly not impossible.
21:55:56 <peter1138> Yeah I know, I'm doing what I tell others not to.
22:03:39 <xarick> awww too good to be true
22:03:51 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
22:05:08 <xarick> unordered_multimap doesn't exist?
22:06:07 <_glx_> you included the header ?
22:06:56 <xarick> cannot open source unordered_multimap
22:10:52 *** HerzogDeXtEr has quit IRC (Read error: Connection reset by peer)
22:11:39 <xarick> im confused by the error
22:12:36 <_glx_> ah it's a static you need to instance it
22:13:15 <peter1138> counts_map needs to be defined as well as declared. (Like always)
22:13:28 <peter1138> You've probably forgotten to change its type signature.
22:14:35 <_glx_> and the error is clear 🙂
22:14:38 *** keikoz has quit IRC (Ping timeout: 480 seconds)
22:14:47 <xarick> not clear at all 😦 but let me check
22:15:50 <_glx_> it just says I saw you declared a static member, but I can't find its definition anywhere
22:16:53 <_glx_> and it does it for every file seeing the declaration
22:18:02 <xarick> it was a bad copy paste
22:18:18 <xarick> copilot made the whole conversion, kinda insane
22:18:48 <xarick> `Industry::counts_map`
22:20:47 <xarick> oh noes... peter wants to merge
22:21:01 <peter1138> No, I made a PR so that if someone else wants to they can.
22:21:24 <peter1138> Otherwise it just gets lost as a branch/stash on my computer.
22:22:43 <xarick> there is a bug in it that I fixed for my tests
22:26:57 <_glx_> of course codeql is right
22:30:42 <xarick> Why are you keeping the Part1 in CheckIfFarEnoughFromConflictingIndustry
22:30:54 <xarick> it's buggy, and not fast
22:31:28 <_glx_> because it's not related to the change
22:36:29 <peter1138> If Part1 is slower than Part2 then I guess it could go.
22:37:47 <peter1138> Vanilla test is still at 50% hehe
22:45:51 <xarick> unordered_multimap disappointed
22:46:58 <_glx_> your storage is complex
22:47:39 <peter1138> Mine might be faster as `std::set<IndustryID>`, didn't try it though.
22:48:27 <_glx_> could as your vector is ordered
22:49:30 <xarick> let me just verify my array of vectors of pairs of towns of vectors of industries again, see if it's really faster
22:49:55 <peter1138> `std::unordered_set<IndustryID>` could give different results though.
22:50:51 <_glx_> for the rare case of equidistant industries
22:51:01 <peter1138> That is, it matters enough times that it's important.
22:51:13 <peter1138> Some times we don't care, but there's no point having both ordered and unordered just for that.
22:51:28 <peter1138> 16000/20000, come on 😄
22:51:49 <peter1138> I fancy testing std::set but can't until this vanilla run is complete.
22:52:22 <_glx_> and non vanilla finished a long time ago ?
22:53:00 <peter1138> Still a long time, but.
22:53:24 <_glx_> compared to 20+ minutes
22:53:41 <xarick> > [2024-11-17 22:45:19] dbg: [misc:0] [GenerateIndustries] 155972662 us [avg: 155972662.0 us]
22:53:41 <xarick> > [2024-11-17 22:51:57] dbg: [misc:0] [GenerateIndustries] 116698659 us [avg: 116698659.0 us]
22:53:55 <xarick> can i have your openttd.cfg for testing
23:00:04 <xarick> the record to beat is 75 seconds
23:00:18 <xarick> but that uses multiple approaches
23:00:34 <michi_cc[d]> peter1138: CBID_VEHICLE_CUSTOM_REFIT maybe?
23:06:45 <xarick> > [2024-11-17 23:05:09] dbg: [misc:0] [GenerateIndustries] 223424100 us [avg: 223424100.0 us]
23:07:22 <xarick> but i suspect it is asserting, gonna test with relwithdebinfo
23:08:13 <xarick> can you share the openttd.cfg ? 42 minutes, what kind of settings are that
23:09:55 <peter1138> peter1138: Only 11 × faster, barely worth it.
23:10:57 <peter1138> With std::set my second test take 5.9 seconds. Uh...
23:12:32 <peter1138> Yeah, std::set is slower.
23:12:39 <peter1138> 50.9s for the vanilla test, vs 20.6s
23:13:12 <peter1138> Iterating std::set is slow, so the "slow" vector insert is not so bad.
23:14:25 <_glx_> vector insert happens in the middle only if some industries are removed then new ones are created in a running game
23:15:31 <peter1138> Oh yes, good point, the vector insert is always at the end here.
23:17:01 <peter1138> So I would guess around 9m30s for the longer test.
23:17:20 <peter1138> Oh no, that's completely wrong.
23:17:29 <peter1138> There's much less industries with ITI because many can't be built.
23:22:39 <_glx_> xarick: without the call stack it's useless 🙂
23:23:13 <xarick> I think, didn't test, but i believe it is
23:23:40 <_glx_> ah it was the OOB array access
23:25:19 <_glx_> hmm but it should have failed on the array access, not on the Get()
23:25:51 <peter1138> I don't use .at() very much...
23:26:08 <_glx_> ah yes OOB with [] is UB
23:26:56 <peter1138> Eh, it's been a few years and old 2 industries have opened, none closed. This test is not vyer testing, hehe.
23:28:25 <talltyler> In ITI, industries never close
23:29:14 *** Wormnest has joined #openttd
23:29:29 <peter1138> Well, vanilla is doing something indeed.
23:29:39 <xarick> > peter's without Part1 on CheckIfFarEnoughFromConflictingIndustry
23:29:39 <xarick> > [2024-11-17 23:26:42] dbg: [misc:0] [GenerateIndustries] 112539982 us [avg: 112539982.0 us]
23:32:07 <peter1138> What are you comparing that with?
23:32:28 <peter1138> Okay, with a 512x512 map, the insert/erase is "nothing"
23:32:28 <peter1138> Each list only has a couple dozen entries so...
23:33:16 <peter1138> 277 total industries. But 512x512 is "small" these days
23:33:33 <peter1138> I didn't save the test maps 😮
23:33:59 <talltyler> If you want to try difficult placement rules with industries that close, ITI Classic is on bananas with similar placement rules, but industries have vanilla production mechanics
23:34:23 <xarick> and then there's Kdtree
23:34:43 <peter1138> Nah, insert/erase cost is not affected by how difficult it is to place.
23:35:15 <_glx_> xarick: what's important is to also have vanilla values, else it's impossible to compare
23:36:04 <xarick> ok, the original, i also have
23:37:46 <xarick> I don't have the total time though
23:38:20 <peter1138> Give it another 40 minutes...
23:38:42 <peter1138> So far it's less than 20µs per insert/erase.
23:39:04 <xarick> I'm off to bed. I'd like to stay, but it's too late here
23:39:44 <peter1138> With fast-forward on, they are happening at a max rate of about 1 per second.
23:39:50 <peter1138> Sometimes much better gap.
23:39:56 <peter1138> I'm going to suggest this is... not noticable.
23:40:19 <peter1138> Although it is mostly inserts at the end at this stage.
23:40:41 <_glx_> insertion in middle are not frequent
23:41:07 <peter1138> I'm waiting for industries to close. I probably need a GS to make this test better.
23:41:21 <peter1138> Or magic bulldozer...
23:42:58 <peter1138> To put that into perspective, having the industry list window open causes 1 second freezes every ~3 seconds.
continue to next day ⏵
