IRC logs for #openttd on OFTC at 2024-11-18
⏴ go to previous day
00:46:51 <peter1138> Hmm, I wonder if the distance check should be fixed to take industry size into account.
00:57:07 <_glx_> using location+half the size ?
01:29:14 *** Wormnest has quit IRC (Ping timeout: 480 seconds)
01:39:32 *** Wormnest has joined #openttd
02:31:52 *** tokai|noir has joined #openttd
02:31:52 *** ChanServ sets mode: +v tokai|noir
02:38:46 *** tokai has quit IRC (Ping timeout: 480 seconds)
02:59:42 *** Extrems has joined #openttd
03:00:14 *** Wormnest has quit IRC (Ping timeout: 480 seconds)
03:18:29 *** debdog has quit IRC (Ping timeout: 480 seconds)
03:21:35 *** Wormnest has joined #openttd
04:20:05 *** D-HUND is now known as debdog
04:20:28 *** Webster has joined #openttd
04:49:53 <DorpsGek> - Update: Translations from eints (by translators)
06:09:29 *** keikoz has quit IRC (Ping timeout: 480 seconds)
07:17:03 <DorpsGek> - Add: summary for week 46 of 2024 (by OpenTTD Survey)
08:44:35 <peter1138> _glx_: Something like that
09:05:17 <truebrain> Something that devices a day in smaller sections
09:05:22 <truebrain> like .... "hours"? Does that work
09:05:28 <truebrain> let's make it 1/24th of a day, that feels right
09:05:35 <truebrain> I can count to 12 on each hand, so 24 will be fine
09:05:39 <truebrain> how to call this service .. hmm ...
09:05:47 <truebrain> sounds like something that would catch on
09:05:50 <truebrain> yeah, let's do that!
09:06:42 <Rubidium> oh, you mean the thing where it's now 17:51?
09:09:03 <LordAro> "I can count to 12 on each hand"
09:09:13 <peter1138> And one value is correct once a day.
09:13:08 <kuhnovic> We need is-it-lunch.openttd.org
09:13:23 <peter1138> Static page that says Yes
09:13:52 <peter1138> andy wants bananas to extra metadata from NewGRFs and export it as a website.
09:14:00 <peter1138> Do you stop at cargo?
09:19:32 <peter1138> Who's reading the API?
09:24:18 <Rubidium> peter1138: all the LLMs
09:49:06 <xarick> peter1138: will you do this?
10:09:54 <xarick> i notice openttd is taking less memory usage on visual studio
10:13:10 <peter1138> What, will I remove that code?
10:13:36 <peter1138> I'm not sure what you're asking tbh 🙂
10:13:57 <peter1138> As far as I know that code is faster than up to 3 per industry type loops.
10:13:59 <xarick> takes half the time on my tests without it
10:14:22 <peter1138> Hmm, I can try it later.
10:14:47 <peter1138> If anything the threshold is wrong as it's not testing GetNumItems() any more.
10:20:35 <peter1138> It's scanning 29x29 tiles isn't it?
10:22:27 <peter1138> It will retest the same industry multiple times if there is more than one industry in each row, though.
10:24:51 <peter1138> 2.45s instead of 5.9s
10:25:32 <peter1138> Let's try the big one again.
10:25:47 <peter1138> Time to beat is 3m 48s
10:30:16 <peter1138> It felt like it took more time :p
10:30:19 *** keikoz has quit IRC (Ping timeout: 480 seconds)
10:31:49 <peter1138> `/* 1. Cheap: Built-in checks on industry level. */`
10:43:07 <xarick> a mix of typelist and kdtree
10:43:24 <xarick> typelist is good for 2 of those functions
10:43:34 <xarick> kdtree for the 2 other
10:44:28 <peter1138> 20.6 to 21s is noise-level margin of error.
10:44:34 <peter1138> But 40% and 60% is not.
10:51:50 <LordAro> unless those two cases are particularly pathological, seems pretty conclusive
10:53:17 <xarick> gonna give up on my town industries pair stuff
10:53:29 <xarick> typelist ends up being better overall
10:54:02 <peter1138> KISS - Keep It Simple, SamuXarick.
10:54:05 <xarick> it only loses in the very specific one on newgrf that wants the layout and town specifics
10:55:04 <peter1138> No, because it's "AI" generated art and therefore inadmissable.
10:55:36 <xarick> oh, it's also winning on FindTownForIndustry
10:55:51 <xarick> the 2 others counterweight massively
10:57:09 <xarick> wait a minute, on second thought... if I were to also use kdtree
10:57:51 <xarick> then ... typelist is not winning
10:58:06 <peter1138> Oh yes, I was reviewing this PR.
10:58:30 <peter1138> (Yes, I demand $work contributors use PRs)
11:01:22 <xarick> The 2 functions where `peter's` excels over `townindustries pairs` are `CheckIfFarEnoughFromConflictingIndustry` and `GetClosestIndustry`, but `kdtree` does even better on these.
11:01:54 <xarick> so.... let me get this straight:
11:04:12 <xarick> best for `CountIndustryMatchingTypeAndLayout` - Town Industries pairs
11:04:12 <xarick> best for `CheckIfFarEnoughFromConflictingIndustry` - Kdtree
11:04:12 <xarick> best for `FindTownForIndustry` - Town Industries pairs
11:04:12 <xarick> best for `GetClosestIndustry` - Kdtree
11:12:18 <peter1138> How large are a kdtree structures?
11:12:32 <peter1138> I expected kdtree to be faster, of course.
11:13:16 <xarick> I don't know how to check these
11:13:49 <xarick> each industry type has a kdtree
11:14:00 <xarick> and there's the global kdtree with all industries
11:14:13 <peter1138> Each kdtree has a vector of nodes, and a vector of indices.
11:14:37 <peter1138> Each node is contains 2 indices, and the element itself
11:15:32 <peter1138> Because it uses size_t, that's going to be 8 + 8 + x bytes, so at least 24 bytes per node.
11:16:07 <peter1138> (I'd be surprised if a kdtree ever gets anywhere close to UINT32_MAX, so those size_ts could probably be changed to uint32_t to save a bit of memory)
11:17:01 <xarick> maybe the global kdtree can be eliminated
11:17:31 <xarick> so 240, with a bit of increased overhead on the already massively fast CheckIfFarEnoughFromConflictingIndustry
11:21:55 <peter1138> It's not a huge issue as it's an existing design, not whatever your pair map pair vector pair stuff was 😉
11:22:59 <xarick> [CheckIfFarEnoughFromConflictingIndustryTypeList] 11302556 us [avg: 0.4 us]
11:22:59 <xarick> [CheckIfFarEnoughFromConflictingIndustryKdtree] 162043 us [avg: 0.0 us]
11:23:42 <xarick> mine was doing 25 seconds here
11:24:14 <exceptik> how long does it take to construct those kdtrees tho?
11:25:54 <xarick> but I can test the total time for generating industries which accounts everything
11:26:21 <xarick> i think kdtree is only slow on removing
11:27:19 <exceptik> could be a good idea to check if kdtrees are faster in the summary, not a specific function you wanted to speed up, but the whole process from gound up
11:28:11 <peter1138> Okay, using uint32_t in the kdtree reduces nodes in station and town trees from 24 bytes to 12 bytes.
11:29:44 <peter1138> On Wentbourne there are 2263 station nodes, so the kdtree is reduced from 54312 to 27156 bytes.
11:29:54 <peter1138> Not huge but maybe worthwhile.
11:31:07 <peter1138> The viewport sign structure is a bit larger but drops from 32 to 20 bytes.
11:32:54 <peter1138> uint16_t could perhaps overflow, as that's nearer the staiton/town limits.
11:36:01 <peter1138> The main issue is that this is dealing with mostly a map-gen problem.
11:36:12 <peter1138> (Industry creation performance, that is)
11:36:26 <xarick> for stations and towns and industries the x y are the coordinates, they can't go over 4096
11:36:40 <xarick> for viewport however, no idea how that part works
11:36:49 <peter1138> I'm not touching the node itself, only the left/right indices.
11:39:25 <peter1138> exceptik: kdtree has an advantage that it's existing code and it was designed to be faster than scanning things. So it's basically designed for (some of) this use case.
11:45:23 <peter1138> I'm "waiting" for Xarick to discover MakeTownHouse has a ForAllStationsAoundTiles()
11:46:42 <peter1138> (And there's a lot of std::set involved)
11:47:53 <peter1138> Probably the worst part is a house can be removed, which then ends up removing a station from the nearby list, but then the house is replaced immediately in the same processing tick, which ends up adding the station back again.
11:48:21 <peter1138> Anyway, std::sets and map scans galore.
11:52:36 <exceptik> peter1138: yeah, i mean like if it has a drawback of very slow preparation time and then is used frequently then the benefits are higher than drawbacks, but if its only used in a preparation time of something bigger, then the benefits become dubious but still could work
11:53:11 <peter1138> Yeah, this stuff IS used during the game for industry generation as well.
11:53:34 <peter1138> But then you don't really notice it becuase it's doing that a couple of times every game month.
11:53:53 <peter1138> I was concerned about the 20µs time of my industry lists.
11:54:25 <peter1138> And then... when I opened the industry list window, the whole game froze for around 1 second every time few seconds, as it was building the list.
11:54:34 <peter1138> So, uh, perspective here.
11:55:05 <exceptik> can't even really parralelize it :<
11:55:16 <peter1138> So you can speed it up in extreme cases at bit, but realististcally those extreme cases aren't really a usable game anyway.
11:55:30 <peter1138> In which case adding the complexity is not worth it.
11:58:05 <peter1138> The industry list window builds a filtered list of industry IDs, and then sorts it.
11:58:26 <peter1138> When there's 20,000 industries the filtering and sorting is not free.
11:59:16 <_glx_> Hmm for filtered version it could use your new lists
12:00:08 <peter1138> It's filtered by a search string, not by type.
12:00:25 <peter1138> Actually you can filter by cargo type too.
12:00:38 <peter1138> But yeah I did look at where else it could be used. That isn't one of them.
12:03:59 *** peter1139 has quit IRC (Read error: Connection reset by peer)
12:04:31 <peter1138> DrawIndustryNames in smallmap, maybe, but would make it complex.
12:05:57 <xarick> should I try Kdtree + TypeList?
12:06:54 <peter1138> You didn't include master.
12:07:04 <xarick> oh, seriously? it's gonna slow down
12:07:12 <xarick> gonna make me wait 1 hours
12:07:28 <xarick> alright, I'll do master during lunch
12:07:49 <peter1138> Without showing master then there's no indication of how much an improvement each method is.
12:08:53 <peter1138> My longest test was 40 minutes.
12:09:16 <peter1138> It's reasonable to test with "normal" cases as well as ultra extremes.
12:09:51 <peter1138> Those extremes are usually not actually playable games anyway.
12:11:14 *** peter1139 has joined #openttd
12:41:53 *** keikoz has quit IRC (Ping timeout: 480 seconds)
12:55:54 <peter1138> Ketchup is industrial waste, it is banned.
13:23:33 <kuhnovic> Is there a reason why invalid orders aren't automatically removed?
13:27:09 <peter1138> Or moving stations.
13:27:53 <peter1138> (And it is easily possible to accidentally remove a whole station)
13:28:25 <kuhnovic> Yeah been there done that
13:28:30 <belajalilija> Yes please don’t delete invalid orders
13:28:35 <peter1138> Also if the invalid orders were removed then you wouldn't know that your orders were maybe not correct.
13:30:10 <kuhnovic> I do see that removed stations get a gray label and stick around for a while until they are fully removed. I guess that's for all the reasons above.
13:30:54 <kuhnovic> I guess it _might_ be safe to delete invalid orders after the station is removed-for-real
13:34:53 <_glx_> once the grey sign is gone it's permanently invalid indeed
13:35:31 <peter1138> What's the purpose if deleting them?
13:35:43 <LordAro> maybe something that differentiates invalid (broken save) vs removed?
13:37:00 <_glx_> though if a station is built on the other side of the map after the grey label is gone, it will "fix" the order
13:42:25 <kuhnovic> I was looking at Wentbourne. It contains lots of vehicles with invalid orders in their order list. Probably from removed stations.
13:42:53 <kuhnovic> (no, I did not remove any buoys yet)
13:45:58 <peter1138> Well, load it in an older version and see if that's due to saveload conversion problem of it is just because that's how it is.
13:46:09 <peter1138> (Or maybe watch the conversion to see)
14:05:24 <peter1138> 1.7.0, so it's not actually REALLY old.
14:05:42 <peter1138> Oh, there's a 1.7.1 update.
14:10:16 <peter1138> Nothing wrong with savegame conversion then.
14:10:40 <peter1138> Old interface scaling is so janky, I'm ashamed.
14:12:16 <kuhnovic> You shouldn't be, because it's fixed now 🙂
14:14:04 <xarick> I'm bad at peercentages?
14:15:08 <xarick> I followed a guidehttps://math.stackexchange.com/questions/1227389/what-is-the-difference-between-faster-by-factor-and-faster-by-percent/2807461#2807461
14:15:11 <peter1138> For the first one, new takes 12.5% the time old takes.
14:17:33 <peter1138> What were the timing with my PR but without kdtree?
14:18:15 <peter1138> Like, it seems logical that you'd compare everywhere so that you can see what bits are improved.
14:18:26 <xarick> do you want with the "fast path" removed or the real PR
14:18:43 <peter1138> Removed I guess, as that'll be pushed later.
14:18:51 <peter1138> I thought you already had the figures.
14:19:19 <xarick> I have, but need to confirm because I'm bad at annotating what I did exactly lol
14:20:13 <xarick> ah, I did with fast path removed
14:20:25 <xarick> let me excel your numbers
14:20:38 <peter1138> Argh, he misunderstood my review and did something else.
14:26:28 <xarick> need to redo the runs to confirm number of industries generated
14:42:53 <xarick> alright, now I need to know how to calculate the % differences like you do
14:44:47 <peter1138> Your "faster" is the same thing, just 100 - x
14:54:31 <xarick> need to 100 - x, I think
15:36:25 <peter1138> The "mismatch" is due to the bug in the map scan, right?
15:37:26 <peter1138> It's interesting that none of the NewGRF tests are compelling.
15:40:13 <peter1138> 420 -> 103 -> 45 is quite a lot.
15:40:46 <peter1138> 894 -> 110 -> 75 is "ok"
16:12:44 <kuhnovic> Is typelist the solution that PeterN made?
16:13:21 <peter1138> That graph is a good representation.
16:13:21 <xarick> without the AreaSearch
16:13:56 <xarick> Master has the area search, untouched
16:14:25 <peter1138> It really does look like the kdtree approach benefits vanilla industries more than NewGRF industries with extra checks.
16:14:41 <peter1138> But vanilla industries don't take as long to start with.
16:14:50 <kuhnovic> Looks like the TypeList is a very good improvement with little code changes or added complexity
16:16:05 <xarick> hmm but if you then exclude master... you get another perspective
16:17:35 <peter1138> What's "TownIndustries", keeping a list of IndustryIDs for each town?
16:18:00 <peter1138> Why is that complex?
16:18:44 <peter1138> Have you done something other than including a `std::vector<IndustryID>` in `Town`?
16:20:13 <peter1138> Okay, so it's not a list of IndustryIDs for each town.
16:21:25 <kuhnovic> xarick: True, but we already went from "geez this takes forever" to "that's reasonable". There's always a way to make the code faster, but often it's not worth the hassle / complexity / etc
16:21:45 <_glx_> and it's only during map generation
16:22:25 <peter1138> xarick: if you generate the map with all these industries and then open the industry list window, is the game playable?
16:22:43 <peter1138> (Hopefuily you saved one somewhere :))
16:22:49 <xarick> yes, I didn't touched that
16:23:02 <xarick> so, it should do the same as before
16:23:09 <_glx_> slow industry list window is due to string handling
16:23:53 <peter1138> I know why it's slow.
16:24:59 <peter1138> If we shave off an extra 20-30% of map generation time with ITI, does that help make the game playable?
16:25:32 <peter1138> The answer is no, it just means we can more quickly make a game that's going to disappoint.
16:25:43 <_glx_> if it was not playable, it will still be indeed
16:26:23 <_glx_> yeah finding out in 5 minutes only (vs 45 minutes)
16:26:32 <peter1138> That's kinda worthwhile 🙂
16:27:12 <peter1138> It does look like it's the kdtree side that's improving things more than the TownIndustries stuff.
16:27:30 <peter1138> kdtree is at least an existing codepath.
16:27:51 <peter1138> Could be interesting to see memory usage figures with those graphs.
16:28:05 *** Wormnest has joined #openttd
16:28:14 <peter1138> e.g. if it doubles memory usage (it won't!), that's a hard no
16:28:20 <xarick> I have no idea how to check that
16:29:08 <xarick> i tried to make the vectors as large as necessary
16:31:01 <xarick> I'm still looking into kdtree, see if it's possible to tweak it for a bail out on first match kind of thing
16:31:02 <peter1138> I wouldn't worry too much about prospectively allocating enough memory. vector is designed to allocate as needed.
16:31:18 <peter1138> If you always know up front exactly how much you need then sure.
16:31:29 <peter1138> But it's a very minor thing that will likely have no bearing.
16:32:44 <xarick> also it would be cool if i could access the elements in the kdtree
16:32:56 <xarick> but they're x, y only, not really useful
16:33:20 <xarick> or wait... they have to store IndustryIDs somewhere
16:33:25 <peter1138> Isn't not designed for it.
16:34:20 <xarick> struct Kdtree_IndustryXYFunc {
16:34:20 <xarick> inline uint16_t operator()(IndustryID iid, int dim)
16:36:06 <peter1138> kdtree.hpp:223 suggests it's not.
16:37:56 <peter1138> Ah yes, the IndustryID is stored, an the xyfunc gets the coerdinates.
16:38:36 <peter1138> So it does not store X/Y data, that's always resolved by looking up the thing.
16:39:11 <peter1138> Except for viewport signs, they do store it in the element.
16:54:15 <mnhebi> Well, that sounds wildly inefficient.
16:56:52 <mnhebi> no the not storing the X/Y
16:57:01 <mnhebi> like what functional reason is there not do that?
16:57:13 <peter1138> Well it's stored with the pool item.
16:57:23 <mnhebi> imma store you with the pool cue.
16:59:55 <mnhebi> don't tell me you guys also look up industry production every time instead of caching it
17:00:04 <xarick> kdtree is missing some "this->"'s
17:00:52 <mnhebi> imma expand your cache then deflate it
17:06:01 <xarick> this->INVALID_NODE is this acceptable?
17:06:15 <peter1138> INVALID_NODE is a static constant.
17:06:30 <xarick> but but... uhh., it's part of the class
17:06:41 <peter1138> It's not a class member.
17:06:47 <mnhebi> this->segfault sounds like prime move here.
17:11:24 <xarick> statics don't let me this?
17:17:33 <peter1138> Don't use this-> with static members.
17:17:43 <peter1138> You can do it but you shouldn't.
17:18:19 <peter1138> Is there an emote for regret?
17:18:40 <peter1138> I suggest you don't make a PR after all..
17:19:22 <xarick> static node_distance SelectNearestNodeDistance(const node_distance &a, const node_distance &b)
17:19:22 <xarick> does it quality as a member?
17:20:00 <peter1138> It's a static method, do not use this->
17:20:40 <xarick> line 258 and 268 call that function
17:26:51 <peter1138> With my other PR I think that function can be made static.
17:27:16 <peter1138> Eh, the ManhattanDistance one.
17:27:35 <peter1138> Actually both of them.
17:28:01 <mnhebi> static functions for everyone!
17:31:19 <xarick> level % 2 vs level & 1 what's faster?
17:31:34 <xarick> or compiler decides what's best?
17:32:39 <mnhebi> can always just asm it and bypass the compiler entirely
17:34:47 <dwfreed> let the compiler figure out the best way to do it
17:35:08 *** HerzogDeXtEr has joined #openttd
17:37:23 <xarick> `if (initial_count < 8) return false; // arbitrary value for "not worth rebalancing"`
17:43:07 *** gelignite has joined #openttd
17:51:45 <xarick> interesting, I could pass Industry instead of IndustryID, not sure if that would be faster
17:53:33 <xarick> std::nth_element mysteriously named std
18:13:56 <xarick> I wonder if all those level % 2 could be set in some temporary variable
18:14:31 <xarick> wow, we got more updates
18:21:15 <peter1138> Only the removal of the 'fast path'
18:29:25 <xarick> `if (ec < nc) nn.left = newidx; else nn.right = newidx;` incorrect code style!
18:42:02 <xarick> is there an advantange in passing size_t as a reference?
18:42:11 <xarick> size_t &next = left_side ? n.left : n.right;
18:43:30 <mnhebi> I mean, passing as reference is always more efficient if you do not need to copy the arguments
18:43:58 *** wallaby2 has joined #openttd
18:44:55 *** wallabra has quit IRC (Read error: No route to host)
18:47:36 <_jgr_> It's not always more efficient, for a size_t or similar small POD it likely won't be
18:48:10 <mnhebi> yes there is the 1% which inverts that maxim
18:57:09 <xarick> DistT ManhattanDistance
18:57:27 <peter1138> * @tparam DistT Type to use for representing distance values.
18:57:51 <peter1138> It's the type to use for representing distance values.
18:59:42 <xarick> abs((DistT)this->xyfunc(element, 0) - (DistT)x), whatever the function returns is then transformed into DistT... but I still don't know if it's signed, unsigned
19:00:02 <xarick> visual studio isn't pointing to anything
19:00:59 <xarick> in a place we have that, in the other we have abs((int)xy[dim] - (int)c))
19:01:15 <xarick> not sure if a consistency issue...
19:01:42 <xarick> or totally unrelated..
19:05:41 <mnhebi> I would think in this case DistT is whatever the distance type needs.
19:06:49 <mnhebi> you'd be better served looking at if the manhattan calculation properly clusters and computes distances between the matrixes efficiently using euclidean distance....that is assuming they do all that >_>
19:11:13 *** gelignite has quit IRC (Quit: Stay safe!)
19:12:04 *** Wormnest has quit IRC (Ping timeout: 480 seconds)
19:12:51 <peter1138> I think you'd be better off doing that yourself.
19:24:48 <mnhebi> Nah man you don't want me around math
19:25:55 <peter1138> I'm not keen on this.
19:26:10 <peter1138> "Disk is OK, one bad sector"
19:26:18 *** Wormnest has joined #openttd
19:28:32 <peter1138> `Read SMART Data failed: scsi error badly formed scsi parameters`
19:29:34 <peter1138> ST3000 sounds like it should be a 3TB Seagate drive.
19:39:40 <LordAro> i'm not sure that's remotely true
19:39:58 <xarick> by old i mean pre-2010
19:42:51 *** Flygon has quit IRC (Read error: Connection reset by peer)
19:48:10 *** Smedles has joined #openttd
19:54:22 <peter1138> Well this one is certainly bad.
20:00:43 <xarick> is it faster to pass std::pair by reference?
20:01:05 <xarick> something something in kdtree using std::pair
20:02:13 <_jgr_> A pair is just a struct of two things, so what are the things?
20:02:34 <xarick> using node_distance = std::pair<T, DistT>;
20:02:52 <_jgr_> So what are T and DistT?
20:03:31 <xarick> node::element and a DistT DistanceManhattan most likely
20:04:44 <peter1138> If you're talking about SelectNearestNodeDistance, it could probably return `const node_distance &`, but it's going to be copied anyway.
20:05:01 <peter1138> I suspect it will end up the same compilation.
20:06:08 <peter1138> Hmm, actually it probalby can't, because one of the parameters, whilst being a reference, is a temporary, so can't be returned as a reference.
20:06:39 <peter1138> So here the copy is needed.
20:07:43 <peter1138> References are good for input parameters, but you need to be careful as return parameters.
20:14:47 <xarick> oh, i found another missing this
20:17:15 <xarick> magic numbers in IsUnbalanced
20:23:51 <xarick> there's several int's out there that are IndustryType
20:24:13 <xarick> the Check Far Conflict one has int too
20:27:16 <peter1138> It's weird how you saw this, and mentioned it, and I said "make a PR" and... 🦗
20:31:20 <xarick> I know nothing about the kdtree but... there is no decreaseunbalanced on Removing an element?
20:31:37 <xarick> they're both increase unbalanced no matter if adding or removing
20:32:09 <peter1138> Removing an element does not balance the tree.
20:43:20 <xarick> people were opposed to my PR's
20:49:13 <xarick> i feel like throwing this code into Copilot and ask him to optimize it lol
20:55:23 *** nielsm has quit IRC (Ping timeout: 480 seconds)
21:04:55 <michi_cc[d]> Oh my, now I have to actually write the docs for it 😎
21:09:58 <xarick> Copilot is looking at Kdtree
21:10:34 <xarick> the entire code fit in the message just shy of the limit
21:14:08 <xarick> well, copilot bugged out, couldn't write his entire response
21:16:29 <xarick> the part that i got he's doing some std::move on the element
21:16:52 <xarick> ` node(T element) : element(std::move(element)), left(INVALID_NODE), right(INVALID_NODE) { }`
21:19:50 <_jgr_> It would make sense if T was expensive to copy (which it isn't)
21:20:15 <xarick> maybe for the viewport struct
21:20:47 <xarick> he's also keen on consting T
21:21:44 *** SigHunter has joined #openttd
21:22:12 <peter1138> Wouldn't it need `T && element`?
21:23:33 <xarick> Copilot replaced this line:
21:23:33 <xarick> `typename std::vector<T>::iterator removed = std::remove(elements.begin(), elements.end(), *exclude_element);`
21:23:33 <xarick> `auto removed = std::remove(elements.begin(), elements.end(), *exclude_element);`
21:23:39 <peter1138> Hmm, apparently not.
21:25:33 <michi_cc[d]> Don't forget for NML that the callback now needs a callback flag (flag 9).
21:25:47 <peter1138> I've got to change all my badge properties now.
21:25:50 <xarick> oh wow, copilot found a duplicated variable...
21:26:03 <peter1138> Should've used constants 😄
21:28:31 <_glx_> xarick: both are exactly the same, except with `auto` you don't have to write the full type
21:31:25 <xarick> maybe they're not duplicate variables... hmm unsure, need to ask a real human 🙂
21:32:51 *** HerzogDeXtEr has quit IRC (Read error: Connection reset by peer)
21:32:59 <xarick> nevermind, the comment says it all
21:33:33 <xarick> node &n = this->nodes[node_idx];
21:33:33 <xarick> node &nn = this->nodes[node_idx];
21:33:38 <xarick> and he removed one of them
21:34:45 <xarick> so basically, some std::move, some const and an auto
21:37:08 <_glx_> would be better if you actually understood what it did 😉
21:43:00 <kuhnovic> Letting Xarick play around with copilot is like letting a kid play with a sharp knife 😛
21:44:02 <xarick> he said something about inlined functions but I don't see any change about it
21:45:25 <xarick> bool IsUnbalanced() const
21:45:48 <_glx_> makes sense, but not vital
21:46:14 <_glx_> of course you need to know the meaning of postfix const
21:46:38 <truebrain> a const that comes after!
21:51:39 <peter1138> Yeah, it's been marked obsolete for years.
21:52:52 <peter1138> It's an index into the cargo translation table.
21:53:11 <_glx_> uses less space than labels
21:53:25 <michi_cc[d]> andythenorth: It is so you can have a "reserved for future use" spot 🙂
21:53:54 <peter1138> Just don't have 255 labels listed.
21:54:38 <peter1138> Speaking of which, variable 18 bits 0..7 will have the value FF if the cargo isn't present in the translation table.
21:54:54 <michi_cc[d]> Good point, I'll add that.
21:55:29 <_glx_> ignore 1D and replace with 2C/2D
21:56:52 <_glx_> it's in the 28/29/32 block
21:58:18 <xarick> static_cast<DistT>, does this make sense?
22:03:30 <xarick> another case where Copilot may be failing, in FindNearestRecursive
22:15:07 <xarick> I'm afraid to update the PR, as it was mostly suggested by Copilot
22:16:11 <xarick> I'm creating a temp branch so you can take a look
22:25:41 *** keikoz has quit IRC (Ping timeout: 480 seconds)
22:26:57 <xarick> where should I expect gains with those std::moves?
22:44:46 <peter1138> As the proposal said, exclude...
23:10:34 <michi_cc[d]> You could still use them as a filter *IF* you can accept that certain waggons might not be available if you use certain industry sets. You can't use them as a filter if you'd consider that worse than the sun going nova or something 🙂
23:14:02 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
continue to next day ⏵
