IRC logs for #openttd on OFTC at 2023-12-30
⏴ go to previous day
00:00:21 <peter1138[d]> `.at()` is the 'safe' way to access it.
00:02:03 <peter1138[d]> Hmm, what's the difference between std::string::c_str() and std::string::data()... I'd assume something to do with nul-termination.
00:02:33 <peter1138[d]> e.g. `fwrite(string.c_str(), string.size(), 1, _iconsole_output_file)` could be `fwrite(string.data(), string.size(), 1, _iconsole_output_file)`
00:02:41 <_jgr_> In older versions of the standard, data() wan't required to do null-termination
00:04:50 <Rubidium> also data() is non-const when the string is non-const, and c_str() is always const
00:24:44 <peter1138[d]> Ah... comments. Oops.
00:27:34 <_glx_> ah yes, I didn't check the comments
00:28:14 <peter1138[d]> Not the only one 😄
00:33:10 *** Wormnest has joined #openttd
03:43:05 *** Wormnest has quit IRC (Quit: Leaving)
04:03:01 *** debdog has quit IRC (Ping timeout: 480 seconds)
04:23:20 *** urdh has quit IRC (Ping timeout: 480 seconds)
05:31:41 *** wallabra has quit IRC (Ping timeout: 480 seconds)
05:52:37 *** wallabra has joined #openttd
06:15:22 *** Tirili has quit IRC (Quit: Leaving)
07:30:52 *** ChanServ sets mode: +v tokai
07:37:50 *** tokai|noir has quit IRC (Ping timeout: 480 seconds)
08:32:20 *** keikoz has quit IRC (Ping timeout: 480 seconds)
09:19:01 *** nielsm has quit IRC (Ping timeout: 480 seconds)
10:36:31 <xarick> I got a crash, some widget crash
10:55:26 <xarick> how useful is my crash report without the dump file?
11:00:04 <xarick> My workflow is ruined for the day
11:18:30 <andythenorth> Horse`Sprites complete for 803 consists; incomplete for 10 consists; 98%` such progress since yesterday 🙂
11:40:48 <andythenorth> _jgr_: plausible to port the split train purchase list? 😛
11:41:00 <andythenorth> it seems to me very unlikely to attract pitchforks
11:43:22 <locosage> split purchase is better than vanilla but far from perfect...
11:53:03 <peter1138[d]> Time to dry off and then fix presumably my bugs.
11:55:44 <xarick> how do I get a dump file from a crash?
12:07:11 <xarick> how do I break an n number of times?
12:07:48 <xarick> for (loop 1) { for (loop 2) { break twice; } }
12:08:23 <_glx_> Dumps from homemade builds are unusable by us
12:09:04 <locosage> you can get stack trace though
12:09:31 <locosage> xarick: with goto, though idk what's openttd codestyle view on that
12:09:40 <Rubidium> though this one is so trivially reproducible, that trying to get that seems wasted effort for now
12:10:05 <locosage> but double break is kinda legit use for it
12:10:40 <Rubidium> I'd use return, or rewrite it
12:10:59 <xarick> ` for (const Vehicle *v : stats.vehicle_list) {
12:10:59 <xarick> for (const Vehicle *u = v; u != nullptr; u = u->Next()) {
12:10:59 <xarick> if (!u->GetEngine()->CanCarryCargo() || !HasBit(cargomask, u->cargo_type)) continue;
12:10:59 <xarick> best_hist = c->old_economy[0].performance_history;
12:10:59 <xarick> best_company = c->index;
12:11:03 <xarick> that break should also break the other loop behind 😦
12:11:50 <Rubidium> as there is nothing that limits you from adding a (sub) function that only contains the two for loops, well except an enormous state in many variables maybe
12:12:12 *** Wormnest has joined #openttd
12:12:37 <locosage> and naming it properly xD
12:13:35 <xarick> or maybe, is there a way to setup a loop that can ignore a break? or make the break passthrough
12:14:56 <peter1138[d]> Put it in its own function so you can return instead of break.
12:15:53 <xarick> that's going to turn ugly 😦
12:16:20 <Rubidium> that code reads like gibberish. The first part of a consist for which the cargo is contained in some set (mask), is the one with the best performance?!?
12:17:28 <xarick> it is a check to see if the company is using any kind of vehicle type
12:18:11 <xarick> "I wanna offer this engine for preview which is a train", "Is this company using trains?"
12:19:43 <xarick> "if the company is using trains, then this is the best company to make the offer"
12:19:43 <locosage> some perf history comparison between companies still seems to be missing
12:20:11 <xarick> and there's some cargo stuff which is ignored in the case of trains, in the code before this part
12:26:29 <locosage> yeah, it does performance comparison before even entering that loop
12:31:36 <xarick> for (const Vehicle *u = v; u != nullptr; u = u->Next()) {
12:33:50 <xarick> these messages are too long for irc
12:34:01 <xarick> is it okay if I do it?
12:34:06 <truebrain> they are too long for Discord even; use a codeblock or gist ..
12:34:26 <xarick> not sure what is codeblock
12:40:23 <truebrain> `error: ‘SHF_COMPRESSED’ was not declared in this scope`
12:40:40 <truebrain> I was about to blame vcpkg for the issue with the nightly
12:40:44 <truebrain> but it seems to be a bit more annoying 😛
12:42:17 <peter1138[d]> Is this related to things glx was fixing earlier, or another breakage?
12:42:54 <truebrain> this time it seems more likely that it no longer works on our old build platform
12:43:16 <truebrain> a year ago breakpad added `SHF_COMPRESSED` support
12:43:26 <truebrain> and recently vcpkg upgraded to a newer breakpad
12:43:31 <truebrain> (which we expect them to do)
12:43:44 <truebrain> just ... `SHF_COMPRESSED` is not a symbol known in our build system 🙂
12:44:24 <truebrain> and they didn't add support for when the symbol is unknown
12:45:03 <truebrain> even their latest version doesn't have it
12:46:49 <peter1138[d]> Hmm, it looks like only SetWidgetDisabledState() ignores the operation if the widget does not exist.
12:47:27 <peter1138[d]> Arguably a window shouldn't be disabling a widget that doesn't exist, but...
12:47:59 <Rubidium> xarick: I wonder whether your algorithm with the cache is really going to be faster in that specific case, as you're going to introduce a lot of non-linear iterations over the vehicle table which might be slower than the current simple linear search
12:49:54 <truebrain> okay .. found a clean solution for that symbol ..
12:50:52 <truebrain> now to test it etc etc .. takes FOR EVER 😛
12:55:40 <truebrain> untested .. going to test it with another change 😛
12:57:13 <Rubidium> xarick: for example, the C++ specification states a O(n log n) complexity for sort, but for small bits (<= 15 elements) that need to be sorted it actually uses an algorithm that is computationally more expensive (with GCC), but because it is more localised/linear it is actually faster. Yes, doing more work can be faster, because accessing memory is *slow*.
13:02:51 <truebrain> right, let's see if my CI coding is better than my C++ coding ..
13:12:51 <truebrain> and while at it, took the effort to do ^^
13:12:56 <truebrain> should make things a bit easier for new developers 🙂
13:13:05 <truebrain> still testing if it actually works etc 😛
13:15:39 <peter1138[d]> New developer!? Surely we need to maintain the exclusive clique of old devs that have been doing it for 25 years!
13:16:52 <truebrain> hmmm .. seems the CI is not doing what I told it to do .. what did I do wrong ..... 😄
13:28:15 <truebrain> so ... who added a test to see that there are no git changes at the end of the CI run? 😛
13:30:18 <_glx_> hmm but for linux release won't wel be back to once installed a lib vcpkg lib will never be updated ?
13:30:49 <truebrain> kinda; depending on the github runner to update once in a while is also a non-optimal approach for that
13:30:55 <truebrain> so I think we should just pin the versions in vcpkg.json
13:30:59 <truebrain> and update that once in a while
13:31:31 <truebrain> but first I need to understand how vcpkg actually caches things, as these local installs are not cached this way ..
13:32:54 <_glx_> yeah it does some caching in vcpkg root before putting stuff in vcpkg_installed in-project
13:33:32 <truebrain> `error: Upgrade upgrades a classic mode installation and thus does not support manifest mode. Consider updating your dependencies by updating your baseline to a current value with vcpkg x-update-baseline and running vcpkg install.`
13:33:36 <truebrain> is the advise of vcpkg itself btw
13:34:25 <truebrain> `Restored 0 package(s) from /home/runner/.cache/vcpkg/archives in 13.8 us. Use --debug to see more details.`
13:34:36 <truebrain> that is the interesting entry for me .. need to figure out when/how it builds those archives
13:35:27 <truebrain> ah .. it does that on its own
13:35:30 <truebrain> now that is much better to cache
13:35:38 <_glx_> anyway "upgrade" was not updating the github cache IIRC (because if you had a match on catch there's no need to save it)
13:35:46 <xarick> crash is gone, confirmed
13:36:06 <truebrain> if we don't cache the installed, but the archives, it should upgrade automatically
13:38:16 <_glx_> but action cache mechanism won't store new stuff if an existing cache was found (unless we use a new key every time)
13:40:21 <truebrain> did test that PR at least; so that is fine to merge 🙂
13:40:30 <_glx_> yeah it's integrated in vcpkg now
13:40:34 <truebrain> off for a while; will do some more testing later 🙂
13:41:41 <truebrain> that moment you keep pressing CANCEL WORKFLOW
13:41:45 <truebrain> but the system is like: nah, I continue
13:43:09 <truebrain> it really REALLY doesn't want to cancel this workflow, lolz
13:43:17 <truebrain> make a cancel button .. make sure it doesn't actually work ...
13:43:57 <_glx_> hmm the suggested way for the env vars feels weird, why don't they use `env:` in the workflow ?
13:44:17 <truebrain> but that method does seem to do what we want ... cache binaries when build, but use the latest version when installing
13:44:25 <truebrain> anyway, something to fiddle with later today 🙂
13:46:40 <xarick> oh, interesting, aircraft vehicle
13:47:02 <xarick> the shadow engine id is the same as the main engine id
13:47:13 <xarick> I thought they had different indexes
14:00:27 <xarick> btw what happened to the river slopes? they're not shadowed anymore?
14:03:31 *** metwo26 has quit IRC (Quit: User went offline on Discord a while ago)
14:05:55 <_glx_> check your blitter in cfg
14:11:17 <talltyler> andythenorth: I second this 🙂
14:18:52 <peter1138[d]> Well that motivation/description text is a bit terse :/
14:20:24 <_glx_> then shadows should work
14:21:17 <peter1138[d]> Are you using Original graphics or OpenGFX?
14:21:43 <peter1138[d]> Use OpenGFX2, that supports it.
14:21:58 <peter1138[d]> The change was for original graphics, OpenGFX provides its own.
14:22:04 <_glx_> ah yes OpenGFX doesn't have the feature
14:22:31 <xarick> strange, I swear I've seen it
14:23:00 <xarick> maybe from screenshots... or I dunno
14:23:12 <peter1138[d]> OpenGFX2 is superior to OpenGFX anyway, even if it's perhaps not finished yet.
14:37:16 <xarick> bah... I'm a terrible coder...
14:38:05 <xarick> break; lines and me adding one more loop in the chain...
14:44:22 <_glx_> you like loops (a little too much) 🙂
14:47:32 <_zephyris> Opengfx2 is "complete" but not bug free at 1x zoom 🙂
14:47:49 <_zephyris> Playtesting needed and bug reports very welcome!
14:53:08 <xarick> be code correct, or code efficient?
14:54:03 <_glx_> often you start with first, then iterate until second
14:54:23 <xarick> I found a break; that interrupts the loop, and the next thing it does is return true, why not just return true right from the break;?
14:55:21 <xarick> is it to avoid repeating return true;?
14:57:07 <xarick> yes, with the original code, but not with 2 for loops
14:57:59 <_glx_> yeah with the extra loop you need to adapt 🙂
14:58:03 <xarick> it needs a bool starting as false that switches to true if a break is issued so it can break both loops
14:58:18 <xarick> only to return true, so why not just return true right away?
15:07:42 <xarick> I've been thinking... there has to be a way to customise the iteration so that it becomes easier, maybe with a helper function or class that does the behind the scenes switching from company to company?
15:10:10 <xarick> in the main code I'd just need to use one loop that iterates over the vehicles, while this helper does the hard work of list switching from company to company and pointing to the next vehicle from the main code iteration
15:16:22 *** Wolf01 has quit IRC (Ping timeout: 480 seconds)
15:16:33 <truebrain> for once CodeQL is right (#11654) 😛
15:16:39 <truebrain> just 1 of the 3 notices
15:24:31 <truebrain> let's see how this works ...
15:24:51 <truebrain> well, rebase before push, but what-ever
15:26:19 <emperorjake> Have there been any previous discussions about turning breakdowns off by default?
15:27:00 <truebrain> okay, gha vcpkg cache is scary good
15:28:01 <truebrain> it even shared results in a single CI run ... one of the jobs was a bit quicker, and another job wanted to install the same package, and it picked that from the cache
15:28:58 <truebrain> it means that both the latest version will always be installed, and that we don't have to manage the cache ourselves. Solves both problems at once _glx_ 🙂
15:29:47 <truebrain> I just need to check if the linux release part manages to store the results in the cache correctly 🙂
15:30:09 <_glx_> a nice vcpkg feature which was not available when we started
15:31:01 <truebrain> emperorjake: most likely; and it is one of those settings, that if you default it to off, nobody is going to put it on. I think it should be on by default 🙂
15:31:25 <truebrain> 14.0 will tell us how many people switch it to off
15:31:57 <emperorjake> Oh yeah, those will be some interesting statistics
15:32:13 <_glx_> default on so people can see how bad it is before switching it off 🙂
15:32:27 <truebrain> _glx_: yeah, kinda 🙂
15:32:28 <reldred> I think both disasters and breakdowns need a sit down and reconcept, turning them off by default I think would be a step backwards.
15:33:53 <_jgr_> In principle you could use a GS to do much of the "disaster" functionality
15:33:56 <xarick> it's a good test for AIs
15:34:16 <xarick> how resilient they are under adverse settings
15:34:22 <truebrain> _jgr_: which loops back to the wish for having more than one GS in a game 🙂
15:34:48 <_glx_> most AIs have issues with basic stuff 🙂
15:35:22 <rau117> ||(yes, yes, yes, no one asked me, again)||
15:35:22 <rau117> If we talk about default settings, how about enabling “ctrl+click to scroll trough all **visible **signals” by default?
15:35:22 <rau117> Anyway, by only path signals are enabled by-default, so “under normal conditions” this will not do anything.
15:35:22 <rau117> But! It will make the setup a little more convenient for those who know that they want to use block signals.
15:35:25 <truebrain> hmm, but odd, bunch of vcpkg dependencies don't show up in the cache .. hmm
15:37:27 <truebrain> haha, lol, CMake triggered "vcpkg install" 😄
15:37:35 <truebrain> did not know it did that 🙂
15:38:01 <truebrain> owh, it is the vcpkg toolscript that does that
15:38:04 <truebrain> makes a bit more sense
15:38:11 <truebrain> does that mean we can skip the install lines?
15:40:37 <truebrain> guess doing it as an extra step is good for visibility in case it fails
15:41:29 <truebrain> except it uses a slightly different folder for it 😄
15:42:23 <_glx_> yeah cmake put them in vcpkg_installed
15:42:31 <truebrain> in the build-folder
15:43:10 <truebrain> `Stored binaries in 0 destinations in 5.2 s.`
15:43:14 <truebrain> that is a long time to not store something, lol
15:47:08 <truebrain> let's try without explicit `vcpkg install` 🙂
15:50:18 <_glx_> maybe there's a way to use the file level `env:` without needing github-script actions
15:50:37 <truebrain> _glx_: do we really care? Their docs suggest using this .. why should we be stubber? 🙂
15:50:58 <truebrain> `Restored 0 package(s) from GitHub Actions Cache in 2.4 s. Use --debug to see more details.` ... I added `--debug` .... lolz
15:50:58 <emperorjake> Thanks, perhaps something similar to JGR's improved breakdowns could be added someday.
15:51:41 <talltyler> A good project for you, I bet 😉
15:52:41 <truebrain> okay, without explicit `vcpkg install` things seem to swim smoothly too
15:53:55 <truebrain> now only to figure out why it doesn't work for Linux Release .. as my `--debug` did nothing!
15:57:08 <_glx_> `mv: cannot stat 'vcpkg-disabled.json': No such file or directory` yeah from build dir that won't work 🙂
15:57:17 <truebrain> already fixed; no worries
15:58:59 <truebrain> I am rather surprised by how good the vcpkg integration works, and the GHA integration on top of that .. cannot complain
15:59:08 <peter1138[d]> I wonder if I can simplify NWidgetToolbarContainer anyway.
15:59:19 <_glx_> hmm what did they break again with icu port ?
15:59:29 <truebrain> don't try to debug my failures
15:59:37 <truebrain> that is not a useful use of your time 😄 (I do appreciate it 😛 )
16:00:37 <truebrain> ah ... curl is too old in the image
16:00:41 <truebrain> `--data-raw` is unknown
16:01:26 <_glx_> yeah the very old linux is not easy 🙂
16:01:43 <truebrain> we had a similar issue with `rustup`
16:07:36 <xarick> i need a BIG savegame, what was the name of that 5000 trains one
16:07:48 <xarick> the one that eats performance
16:09:59 <xarick> I have it, I just forgot the name
16:12:19 <xarick> TOCC("GroupStatistics::UpdateProfits", 1);
16:12:36 <xarick> called on the start of new year
16:12:43 <peter1138[d]> Hmm, shared_ptr might make things simpler.
16:15:16 <truebrain> to be or not to be 😛
16:21:57 <truebrain> okay, that PR now does too much in a single PR, but I will fix that when I know it works 🙂
16:22:18 <truebrain> the cache does make installing dependencies really fast; so we have that going for us 🙂
16:25:30 <truebrain> `Stored binaries in 1 destinations in 274 ms.` .. owh yeah, that works 😄
16:25:43 <truebrain> also makes testing of the release flow faster btw 😛
16:25:49 <truebrain> it is only slow once 😄 Even if it fails 🙂
16:32:14 <peter1138[d]> UpdateProfits is called once a game year. I think you've focused on the wrong situation there.
16:34:03 <_glx_> truebrain: huge improvements as cache was not stored on failure before
16:34:34 <_glx_> and failures are common when testing changes
16:34:37 <truebrain> I am trying to understand what `core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || '');` is doing exactly .. and it kinda baffles me, I have to admit ..
16:35:55 <_glx_> I think it's similar to `ACTIONS_CACHE_URL: ${{ vars.ACTION_CACHE_URL }}` in env but it's hard to find doc
16:36:32 <truebrain> the weirdness is that `github-script` doesn't document `process.env` .. so I think it is just `ACTIONS_CACHE_URL: ${{ env.ACTIONS_CACHE_URL }}`
16:38:11 <_glx_> it's supposed to set env vars (that's how I understand the text in vcpkg doc)
16:38:20 <truebrain> yes, but with the value of the env 😛
16:38:24 <truebrain> that is what is so trippy
16:43:59 <_glx_> looks like a special env var only available for some actions
16:44:22 <truebrain> that doesn't sound like GitHub to do, honestly
16:45:18 <_glx_> and that forces a workaround to export it for others
16:46:56 <truebrain> looking in the source of runners
16:47:08 <truebrain> only node-scripts and containers get these variables
16:47:10 <truebrain> shell scripts do no
16:47:59 <truebrain> yup, a container gets them too 😛
16:48:04 <truebrain> that is just a bit funky
16:48:20 <_glx_> so linux release should have it
16:48:30 <truebrain> not sure if those kind of containers get them
16:48:33 <truebrain> docker-based actions do
16:48:36 <truebrain> and node-script actions do
16:48:42 <truebrain> composite and shell actions do not
16:49:56 <truebrain> but okay .. so this should now be all green .. after that, I will split it up into a few smaller PRs, for the "git blame" we will have to do some day 😄
17:06:36 <truebrain> no real need for that 🙂
17:06:41 <_glx_> but it does a little too much yeah
17:10:59 <truebrain> okay, small commits now! Let me know if you want them in separate PRs
17:12:23 <_glx_> as long it's not squashed it should be fine for blame
17:15:27 <LordAro> anyone feel like maybe we need some more unit tests for the Window code?
17:15:49 <truebrain> how long is your holiday? 😄
17:18:57 <truebrain> right, CI is doing what it should be doing .. need to wait for the release to finish to double-check that .. that it actually build static executables 🙂
17:19:53 <peter1138[d]> SetFocusedWidget's checks were very strange.
17:21:39 <truebrain> okay, release-flow is also doing what I expect .. we would need to check nightlies after this PR merges, but .. it is looking good 🙂
17:22:09 <peter1138[d]> LordAro: this is all runtime stuff that depends on interaction with the window. Difficult to unit-test.
17:22:37 <truebrain> owh, I thought he was just volunteering 😄 My bad 😉
17:22:46 <LordAro> i would never do such a thing
17:23:16 <LordAro> though for some reason i am currently playing around with removing ZeroedMemoryAllocator from a few more places
17:23:35 <peter1138[d]> This is actually a bug in how NWID_MATRIX works.
17:23:48 <LordAro> WindowDesc sets all its members explicitly already, so that one's easy
17:23:50 <peter1138[d]> It was hidden before, but still a bug.
17:24:38 <peter1138[d]> It puts a temporary value in the top 16 bits of widget->index
17:25:10 <peter1138[d]> So... time to change. Fortunately I was already considering changing, so now I have a good 'excuse'.
17:25:53 <peter1138[d]> Basically the same issue that you had, yeah.
17:26:33 <peter1138[d]> But, does the userdata live on the widget that does special things, or the widget that is being asked to draw...
17:26:49 <truebrain> now that is a question 😄
17:27:35 <truebrain> the latter is I guess easiest in most cases
17:28:07 <truebrain> either way won't solve my dropdown issue btw 😛 But that is a completely different story 🙂
17:28:23 <peter1138[d]> I think it's related.
17:28:49 <truebrain> somewhat ... the find out where to draw the dropdown, it uses the widget-id .. so you can't have 2 dropdowns with the same id 😛
17:28:55 <peter1138[d]> nwid_matrix sets bits in its child widget->index which tells the child what to draw.
17:29:02 <truebrain> we would need to change the dropdown to use userdata too, if we want to solve that 🙂
17:30:08 <truebrain> oeh, bit late to get food, quick, time to get food!
17:30:19 <truebrain> _glx_: PR is ready for review, all tests are green 🙂
17:30:44 <xarick> (!v->IsGroundVehicle() || v->IsFrontEngine()) isn't this equivalent to v->IsPrimaryVehicle() ?
17:31:47 <xarick> if it's a ship, does it have a front engine?
17:32:20 <_jgr_> No, because of aircraft shadows/rotors, but IsPrimaryVehicle is probably suitable
17:32:38 <xarick> awesome, that means deleting code
17:33:33 <xarick> I wasn't even aware of this feature
17:47:35 <xarick> it works! I didn't break it
17:48:21 <xarick> i just don't know of any AI that uses sub-groups
17:48:32 <xarick> I wanted to see the gains!
17:49:05 <peter1138[d]> xarick: If it's a ship, then it's not a ground vehicle, so the frontengine test is not called.
17:50:47 <xarick> CivilAI colours stuff, not sure it does it on a group level
17:56:45 <peter1138[d]> Not exactly userdata after all.
18:03:27 *** Flygon has quit IRC (Quit: A toaster's basically a soldering iron designed to toast bread)
18:07:40 <peter1138[d]> /me tries something
18:12:59 <xarick> CmdAddSharedVehicleGroup - my mental gymnastics trying to decipher what this is doing
18:13:56 <xarick> is it safe to do group vehicle movements here?
18:14:12 <_glx_> hmm I need to modify my linux preset
18:14:59 <peter1138[d]> It adds all vehicles with shared orders to a group.
18:15:36 <truebrain> So matrix is using the same trick as I am for the social tab .. so it must be good 😛
18:16:02 <peter1138[d]> Are you stuffing it into the index? 😄
18:21:40 <_glx_> ok wsl works again (after I excluded the vcpkg.json from rsync)
18:22:52 <xarick> I wonder if this CmdAddSharedVehicleGroup is missing a break;
18:23:00 <_glx_> was simpler than trying to find why icu fails to build
18:23:26 <xarick> is it intended to keep cycling all the vehicles of a group?
18:26:43 <xarick> the comments kinda imply it's just for the first vehicle it finds
18:27:20 <peter1138[d]> Now try changing widget index, muwahahaha
18:32:10 <_glx_> hmm yeah, I think it will iterate the shared list multiple times
18:32:40 <_glx_> for nothing as they are moved to the group on first pass
18:34:29 <peter1138[d]> You might have vehicles with different shared orders in a group.
18:34:57 <peter1138[d]> But it's not clear to me what it is meant to be doing.
18:35:20 <peter1138[d]> "from a group" the doxygen seems like it should be "to a group"
18:35:49 <xarick> got a crash here, maybe... it's bugged, or was it my changes...
18:36:01 <xarick> let me test with original code
18:36:03 <peter1138[d]> But it's not, because it checks that v->group_id is the group, so...
18:36:28 <peter1138[d]> I guess it moves all shared vehicles of vehicles in a group, into that group.
18:36:32 <_glx_> for each vehicle in the group, it adds shared to the group
18:37:02 <peter1138[d]> Technically it's changing the iterator while iterating 😄
18:37:50 <xarick> i tested like this: buy 4 ships, the first 2 ships share orders with each other, the last 2 ships share orders with each other
18:38:02 <xarick> create a group, move 1 ship to it
18:38:07 <xarick> now add shared orders, crash
18:38:33 <DorpsGek> - Update: Translations from eints (by translators)
18:38:48 <xarick> with my changes, that is
18:39:46 <_glx_> as usual you PR diverged from the original intention 🙂
18:41:20 <xarick> dang, can't use the cache here it seems
18:42:09 <_glx_> of course you can't, the groups and the cache are modified by the function
18:45:59 <xarick> it does not exist in the cache
18:46:08 <xarick> yeah, the DO command moved it out
18:46:26 <xarick> i see, makes sense that it works on master, that list is fixed
18:54:31 <xarick> I see a way to do this (maybe)
18:55:36 <xarick> copy the list to a temporary list, and use the temporary list for iteration
18:55:58 <xarick> probably bad for performance
18:56:15 <xarick> but ... I wanna test it either way
18:58:44 <peter1138[d]> Hmm, well, 6000+ miles is okay, but on my *first* full year of cycling I did over 8000 😮
18:59:16 <peter1138[d]> And last year was better, meh.
19:00:07 <peter1138[d]> I guess cycle commuting easy adds miles without 'trying'.
19:14:44 <truebrain> peter1138[d]: Noooooooo! Who approved that?! You murdered my PR 😛
19:15:15 <truebrain> Now I need to find another trick 😛
19:15:21 <truebrain> Or fix it properly .....
19:16:56 <truebrain> it does make for a challenge; but having it const is a tiny bit better 😄
19:17:08 <truebrain> dropdowns are just a bit annoying that they do depend on an ID
19:18:16 <peter1138[d]> I'd borrow from NWidgetMatrix, possibly even share the same bits.
19:18:32 <truebrain> not sure how that will solve my Dropdown issue 😛
19:18:46 <peter1138[d]> What's the issue with dropdowns?
19:18:52 <truebrain> I don't want to dynamically make the widget; I like my static declaration 😄
19:19:00 <truebrain> dropdowns need unique IDs to function
19:19:08 <truebrain> they self-reference a lot based on the ID
19:20:58 <peter1138[d]> Hmm, add userdata to dropdowns.
19:21:01 <truebrain> owh, I removed the dropdown for now .. I forgot about that; so I don't care! 😛
19:21:09 <peter1138[d]> So that it gets passed back when the dropdown is closed.
19:21:14 <truebrain> but yeah, we need either userdata, or some hierarcy information
19:21:28 <truebrain> but I completely forgot I removed it for now; so LALALALA, I said NOTHING 🙂
19:22:36 <peter1138[d]> To be honest, I was thinking even "int index" is a bit awkward, because it assumes every response value is an int.
19:22:53 <peter1138[d]> Well, forces even.
19:25:58 <peter1138[d]> But using `void *` for this stuff smells of C :p
19:26:19 <truebrain> I was thinking it might help if widgets can find their "parent"
19:26:27 <truebrain> as that is kinda often what you want/need
19:26:34 <truebrain> so the draw routine doesn't go from Window -> Widget
19:26:53 <peter1138[d]> Draw already goes from Window -> Widget -> Widget -> Widget...
19:27:01 <truebrain> yeah, sorry, Drawing was easy to fix 😛
19:27:22 <peter1138[d]> OnClick does too.
19:27:39 <truebrain> no, OnClick works on the ID
19:27:43 <truebrain> hmm, I need to word this better 😛
19:28:19 <peter1138[d]> Yes they pass the widget id, but it goes through GetWidgetFromPt() which passes through the widget tree.
19:28:40 <peter1138[d]> Which is how matrix works, it sets the current element during that call.
19:31:38 <peter1138[d]> I should look at the PR to see what you are doing 🙂
19:33:22 <peter1138[d]> Oh, you are actually creating separate widgets instead of reusing the same widget like NWidgetMatrix.
19:33:31 <truebrain> right, so there were 2 things I ran into: the first is that a widget can't find its "parent". It needs to do that based on an ID. Which means making generic reusable components is kinda tricky
19:33:39 <truebrain> ideally I just do a: find first parent of type
19:33:47 <truebrain> there is something sort-of like that, but not really, from what I could find
19:34:03 <peter1138[d]> In that case you have an ID but you don't actually know what it refers to.
19:34:11 <truebrain> for the matrix you now do the same thing: keep an index on the matrix, and the widget finds that matrix back to drag out the index
19:34:46 <truebrain> for drawing etc that is doable to hack it in there, but for some other events we are not that lucky, which makes for a bit annoying code
19:35:05 <truebrain> the other thing I ran into, that you kinda want events to be handled locally. With that I mean: an OnClick is handled on the window, but I want to handle it in my own Widget
19:35:10 <truebrain> so now on the Window I need to redirect it to the Widget
19:35:12 <peter1138[d]> I think dynamically creating widgets vs reusing the same widget is a fundamental difference here than means all my ponderings about how you might do it are moot 🙂
19:35:15 <truebrain> which introduces a lot of boilerplate
19:35:39 <truebrain> basically what I am talking about is creating your own widget you can reuse, so you can dynamically create multiple 😄
19:35:53 <truebrain> but from what I can tell, the window system is not far away from making these things possible
19:36:06 <truebrain> your changes really helped a lot to make more clear what is actually going on 🙂
19:36:29 <truebrain> and I think with these 2 changes I suggest, something like a Dropdown becomes a "custom" widget, that is easily reusable
19:36:45 <truebrain> without having it assigned an ID or something
19:36:51 <truebrain> but I might miss some other important parts of the Window system 😄
19:37:02 <peter1138[d]> If your child-widgets had something (userdata!) that referred to the plugin, then it would be quite simple I think.
19:37:14 <peter1138[d]> (Although perhaps not the design you are thinking of)
19:37:20 <truebrain> that would help for sure
19:37:31 <truebrain> I was more thinking about a function that does tree-walking to find component of type N
19:37:40 <truebrain> I think that alone would solve a lot of these issues
19:37:55 <truebrain> userdata is the reverse of that: push to the widget where that parent is 😄
19:38:03 <truebrain> 2 sides of the same coin, I think
19:38:06 <peter1138[d]> GetWidgetOfType() exists but probably not as you need.
19:38:23 <truebrain> no, or at least, I couldn't get it to work how I imagined it 😛
19:38:32 <truebrain> it was only on the Window?
19:38:59 <peter1138[d]> No, it's on NwidgetBase.
19:39:06 <truebrain> ah, no, it went parent->child
19:39:11 <truebrain> I am looking for child->parent 😄
19:39:29 <peter1138[d]> There's no record of parent, indeed.
19:39:45 <truebrain> as that is basically what you did for the matrix now too
19:39:50 <truebrain> a child->parent link .. but via an ID 🙂
19:39:52 <peter1138[d]> Could probably be added easily enough but it's never been needed before.
19:40:18 <peter1138[d]> Yeah I guess. Because we know who the parent is, even if the code doesn't.
19:40:36 <truebrain> so many roads to Rome 😄
19:40:39 <peter1138[d]> It is prone to error, because I could have used the wrong widget ID.
19:40:49 <truebrain> I might fiddle with it a bit next year 🙂
19:41:08 <truebrain> but yeah, it seems that if we just add the parent to each widget, we are already there for most part 😄
19:41:14 <peter1138[d]> Anyway, we could potentially walk the tree to find the child, and then return the parent that way without storing the parent explicitly.
19:41:17 <truebrain> `GetParentOfType` 🙂
19:41:37 <peter1138[d]> Or "closest()" as javascript calls it 😄
19:41:47 <peter1138[d]> Yes, fairly expensive, but how often is it used...
19:41:55 <truebrain> redraws happen A LOT I noticed 😦
19:42:00 <truebrain> several times per second
19:42:02 <peter1138[d]> Ah redraw, true.
19:42:10 <truebrain> but a pointer is cheap
19:42:15 <truebrain> not many Windows are active at any given time
19:44:03 <peter1138[d]> GetParentOfType then has a simple list to traverse.
19:44:09 <truebrain> the other thing I mention, that Widgets can handle events instead of Window, could also make code a bit cleaner / easier to read, as for example no massive switch statements would be needed
19:44:39 <truebrain> just the code needs to be somewhere .. so many more smaller functions would exist
19:44:45 <truebrain> so not sure if that is actually better 😛
19:45:08 <truebrain> but I can imagine you can do a SetOnClick, like SetPIP, instead of having it in a massive switch
19:45:24 <truebrain> but that very soon starts to mix styling with functionality
19:45:28 <truebrain> and currently that is well split
19:46:28 <peter1138[d]> Then you end up needing to deal with static methods.
19:47:40 <peter1138[d]> And then you probablky need to cast from Window * to the window tyep...
19:48:12 <truebrain> that is what templating is for 😛
19:55:25 <truebrain> hmm .. come to think of it, it would mean that with both additions, you can do away with almost all widgetIDs 😛
19:55:30 <truebrain> in most cases, you won't need it anymore then 🙂
19:55:55 <truebrain> on one hand it is time related, on the other hand it looks weird
19:56:15 <truebrain> I never understood why they made then/than 😛
19:57:09 <xarick> > /* For each vehicle belonging to the group id_g
19:57:09 <xarick> > * add all vehicles sharing orders with it to that group */
20:00:58 <xarick> hmm, I could const that list, brb
20:17:52 <peter1138[d]> Huh, how does protected work?
20:18:13 <peter1138[d]> NWidgetBackground derives from NWidgetBase, yet it can't set a protected member of NWidgetBase...
20:19:50 <peter1138[d]> Oh because it's a member of a different widget.
20:30:42 <peter1138[d]> ```Out of memory: Killed process 805212 (ninja) total-vm:22546548kB, anon-rss:22540360kB, file-rss:1780kB, shmem-rss:0kB, UID:1000 pgtables:44168kB oom_score_adj:100```
20:34:19 <peter1138[d]> Hmm, rearranging scrollbars to be parent widgets might be interesting.
20:35:11 <peter1138[d]> But I probably not all layouts are quite right to support that.
20:35:19 <peter1138[d]> (Which is a general issue with GetParentWidget.
20:36:16 <peter1138[d]> `const NWidgetMatrix *matrix = this->GetWidget<NWidgetMatrix>(WID_BO_OBJECT_MATRIX);`
20:36:29 <peter1138[d]> becomes `const NWidgetMatrix *matrix = this->GetWidget<NWidgetBase>(widget)->GetParentWidget<NWidgetMatrix>();`
20:36:57 <peter1138[d]> (Until we either pass `Widget *` around more or move more widget handling to widgets)
20:37:31 <peter1138[d]> But, uh, I built this on the unique_ptr branch.
20:38:31 <_glx_> hmm if scrollbar is parent it would allow scroll and shift+scroll to pass to vertical or horizontal
20:39:42 <peter1138[d]> Oh right, because SetScrollbar only allows for one, and that's the one that scroll wheel is passed to.
20:41:28 <peter1138[d]> Ah we have NWID_HSCROLLBAR and NWID_VSCROLLBAR, but they are both NWidgetScrollbar.
20:42:06 <peter1138[d]> Maybe we should split them 🙂
20:43:07 <peter1138[d]> Yeah, like we already do for horizontal and vertical containers.
20:43:54 <peter1138[d]> Hmm, if a scrollbar becomes a container, then you can potentially scroll anything.
20:44:11 <peter1138[d]> Which was a goal of mine ages ago.
20:44:51 <peter1138[d]> And it can still work the current way to avoid needing to change all the code at once.
21:02:05 <peter1138[d]> All tests passed (2960 assertions in 69 test cases)
21:02:09 <peter1138[d]> Too many unit tests?
21:02:15 <truebrain> NEVER change the amount of test cases again
21:02:17 <truebrain> this is the final form
21:05:48 <peter1138[d]> This should really be in a separate test case but it would need to repeat code that is entirely the content of an existing test. So that feels weird 😮
21:16:10 <peter1138[d]> ^ might help you truebrain
21:16:41 <truebrain> now we need a shorthand for the `GetWidget` 😛
21:17:23 <truebrain> peter1138[d]: `parent_widget` or just `parent`?
21:17:44 <xarick> some crash in CopyOutDParam
21:19:51 <peter1138[d]> Hmm, overly verbose?
21:20:23 <xarick> I didn't touch strings... so I doubt it's my fault
21:21:48 <peter1138[d]> Yeah we have "child" and not "child_widget"
21:22:49 <peter1138[d]> Not for much longer 😉
21:23:10 <truebrain> did you fix CodeQLs warning? 😄
21:23:49 <peter1138[d]> Dunno but now I want to rename it from "widgets" to "children" or something.
21:24:51 <peter1138[d]> Oh, GetWidget uses "NWID" as the template type.
21:28:34 <peter1138[d]> Oh a bit late to open another beer.
21:30:42 <xarick> I'm unable to reproduce the string crash
21:32:24 <peter1138[d]> Well, it doesnt' compile right now 😮
21:33:03 <peter1138[d]> Because I did a silly.
21:35:13 <peter1138[d]> Thanks, and sorry!
21:35:24 <peter1138[d]> So glad the svn days are gone.
21:35:27 <truebrain> I didnt see it in review 😛
21:36:32 <peter1138[d]> Tweaking the widget system, bit by bit.
21:42:37 <peter1138[d]> Oh it's rebased onto 11659 of course.
21:42:59 <peter1138[d]> I should check the warnings this time 😄
21:43:52 <truebrain> codewise it looks fine
21:44:04 <truebrain> I am amused you use `up.get()` and not `*up` 😄
21:44:11 <truebrain> (up = unique pointer variable)
21:44:37 <peter1138[d]> I never said I was a good coder.
21:44:53 <truebrain> just most people are lazy; I appreciate your effort in doing more letters 🙂
21:45:03 <truebrain> so I am not against your code at all 🙂
21:56:01 <xarick> you're not a good coder? damn, I really must be crap
22:00:13 <peter1138[d]> Hmm, some cases can't be auto, but I can do most.
22:01:46 <peter1138[d]> Hmm, seems `*nwid` isn't the same as `nwid.get()` anyway.
22:03:16 <peter1138[d]> Well that's awkward.
22:04:03 <truebrain> bit weird also, that it considers `this` a temporary value?
22:04:45 <peter1138[d]> It's correct, if you had a widget on stack instead of heap. I guess.
22:04:58 <peter1138[d]> But then you wouldn't be able to add it to the widget tree anyway.
22:05:30 <peter1138[d]> So my new rule, don't enable automerge :S
22:06:05 *** nielsm has quit IRC (Ping timeout: 480 seconds)
22:06:14 <truebrain> we can also make CodeQL mandatory?
22:13:36 <peter1138[d]> Okay, the solution to that even happening is to make the constructor private and use an factory method to create an instance instead.
22:17:43 *** Wormnest has quit IRC (Quit: Leaving)
22:17:46 *** keikoz has quit IRC (Ping timeout: 480 seconds)
22:18:02 <peter1138[d]> That is something I could do, it might then avoid std::make_unique everywhere
22:24:03 <peter1138[d]> Arguably we already mostly use a factory system, but it's not enforced as such.
23:12:47 <peter1138[d]> You can't use std::make_unique with private/protected constructors, of course.
23:15:44 <peter1138[d]> Oh, you can, but... urgh.
23:21:00 <xarick> I touched a "network" file
23:29:54 <Eddi|zuHause> have you washed your hands afterwards?
23:33:21 <xarick> NetworkAdminUpdate... hmm I don't know
23:33:46 <xarick> I don't know how to trigger a call to NetworkPopulateCompanyStats
23:34:25 <LordAro> something to do with the admin port, i would assume
23:35:57 <xarick> I'm gonna assume this code works
23:40:35 <xarick> ServerNetworkAdminSocketHandler, it's empty... I am not Admin apparently
23:45:42 *** keikoz has quit IRC (Ping timeout: 480 seconds)
23:55:12 <xarick> could someone test `NetworkPopulateCompanyStats` for me? I don't know how to setup Admin stuff
continue to next day ⏵
