IRC logs for #openttd on OFTC at 2022-10-25
⏴ go to previous day
00:00:33 <glx[d]> when we use inline the declaration and definition are in the same place
00:00:50 <glx[d]> because it's the only time it makes sense
00:01:35 <Arastais> petern: yea, so that's a good reason for a fix commit and is related to coding stlye.
00:01:35 <Arastais> Should I make a fix commit to remove it then?
00:02:14 <TallTyler> glx[d]: Would you consider it wrong in the PR? Or should it be included and then changed as a codechange PR later?
00:03:03 <glx[d]> the PR needs CDECL, else it would need to change all other locations
00:04:00 <glx[d]> CDECL can be removed, but it's not in scope of the PR
00:05:53 <petern> Code style doesn't quite describe an incorrect inline attribute that prevents compilation with compilers 🙂
00:28:38 <Arastais> petern: I wouldn't really call it incorrect 😛 it's part of the standard so the XCode compiler failing in that case may be a bug. Other compilers don't seem to have a problem. So I would say it comes down to preference/style if you actually do intend for it to be inlined.. And separating and inline function definition and declaration is not uncommon in general, especially with member functions. There's no strict rule that says if
00:28:38 <Arastais> it must be defined in the header.
00:28:38 <Arastais> Regardless, would you like me to make a fix commit and remove the `inline` specifier if you think it shouldn't be hinted as such? I kept it around because it was there before, but I thought it was there to hint that it should be inlined, not solely because it was both defined and declared at once before. so the I guess the golden question is do you intend for the `GetSmallMapOwnerPixels(...)` to be inlined?
00:32:28 <petern> > Note: It’s imperative that the function’s definition (the part between the {...}) be placed in a header file, unless the function is used only in a single .cpp file.
00:37:35 <petern> Somehow they all say the same thing...
00:38:53 <petern> It was static inline before, which was allow because it was only used from that source file.
00:39:05 <glx[d]> for me it's just logic, the only valid location for `inline` is where definition is also the declaration
00:39:48 <petern> Arastais: I'm not actually asking you to do anything, just trying to get you to see that it is wrong, it's not a matter of opinion.
01:08:36 <glx[d]> and for a compiler it end up as being not defined in screenshot.cpp translation unit
01:14:08 <glx[d]> btw in c++ `inline` means "multiple definitions are permitted" rather than "inlining is preferred"
01:18:44 <petern> I've added a load of inline member functions in my scaling branch...
01:22:27 <Arastais> petern: Well, I was referring more to everyone's earlier points, specifically what michi said and what you were saying about "We shouldn't hint a function as inline when it's used not-inline", which I thought you meant in terms of coding style. I think I also misunderstood what you said here ^. Not to mention I don't use inline functions outside of their same translation unit very often tbh, so I didn't think about what you're talki
01:22:27 <Arastais> that inline functions don't have to be defined and declared at once, and can be defined in a separate file, but I guess i overlooked the important caveats.
01:22:27 <Arastais> However, I don't think that what you're saying is what's going on in this specific case, because if it were then all the CI checks shouldn't have passed. If anything it should for sure throw linker errors in debug mode, where inlining is not optimized. Now that I think about it, it's more similar to what glx[d] is saying. I think that because there's no definition in `screenshot.cpp`, the compilers are either doing whatever they wan
01:22:27 <Arastais> where it doesn't matter or using the definition from the other TU) or its undefined behavior. So it seems gcc and msvc don't complain, but that would explain why XCode says the definition doesn't exist, because it's missing from the other TU so I guess XCode has a differnt way of handling that. For that reason, the `inline` specifier should be removed. The compilers behave inconsistently for one reason or another.
01:38:14 <Arastais> petern: In fact, when I try a minimal example in both debug and release in msvc, it works fine. So while it is wrong, and wile what you're saying here ^ is correct, I don't think that's what going on here.
01:45:28 <petern> With g++, it compiles without -O, but not with -O.
01:45:49 <petern> (Sorry, "it" being a minimal test case, not OpenTTD)
01:46:12 <Arastais> petern: what's the exact error?
01:46:25 <petern> `b.cpp:(.text+0x15): undefined reference to `foo(int)'`
01:47:31 <petern> Because b.cpp can only "see" the declaration, not the definition.
01:47:47 <petern> And you can't declare it inline without the body.
01:47:48 <Arastais> petern: exactly, so its not a linker error meaning it's not your previous reasoning. It must be because it's missing the definition all together in the other translation unit
01:48:07 <petern> It is entirely what I said.
01:51:28 <glx[d]> it's still a linker error 🙂
01:52:04 <glx[d]> missing definitions are reported when linking
01:52:09 <petern> undefined reference is most definitely a linker error.
01:54:01 <petern> And just because the wording isn't the same across different environments doesn't mean it's a different issue.
01:56:50 <Arastais> huh, mb then, in the minimal example in msvc, if I reference the inlined function from another TU outright i get LNK linker errors, but if I reference a funciton from another TU that uses said reference function instead (similar to what openttd does), it either works or I get C3861 identifier not found depending on the optimization level.
01:57:01 *** _aD has quit IRC (Quit: leaving)
01:57:24 <Arastais> so it's different for me in msvc, but tbh it doesn't matter it should be removed naywa
01:58:32 <glx[d]> I'll stick to "use `inline` only if declaration and definition are in the same location" 🙂
01:59:41 <glx[d]> so `static inline` in .cpp or `inline` in header
01:59:47 <petern> You might find this code was duplicated because not allowing it to be inlined (i.e. with no LTO) causes a severe performance issue...
01:59:57 <petern> But I think we all have LTO these days.
02:00:12 <petern> I hope I do, it takes long enough to compile & Link...
02:00:47 <Arastais> sorry for all the trouble lol, I misunderstood what you were saying near the end
02:21:12 <Arastais> sorry - branch was named incorrectly. Let me redo it
02:22:41 <Arastais> you should probably delete #10109
02:22:41 <glx[d]> branch name doesn't really matter
02:22:59 <Arastais> well, ok u can reopen it then
02:26:02 <Arastais> ok, sorry about that, good to go
02:26:11 <Arastais> ok, sorry about that, good to go
02:33:57 *** Wormnest has quit IRC (Quit: Leaving)
03:02:16 *** debdog has quit IRC (Ping timeout: 480 seconds)
03:09:36 *** D-HUND is now known as debdog
03:32:19 *** debdog has quit IRC (Ping timeout: 480 seconds)
03:38:30 *** D-HUND is now known as debdog
05:13:26 <Bouke> Rubidium: `dsym` files (Debug SYMbol file) would be the equivalent of pdb files for macOS builds. But I think the increased binary size is fine, so I’ll just leave it as-is.
05:14:11 *** WormnestAndroid has quit IRC (Read error: Connection reset by peer)
05:14:31 *** WormnestAndroid has joined #openttd
06:26:00 *** sla_ro|master has joined #openttd
06:36:11 *** debdog has quit IRC (Quit: Initiating getting-the-hell-out-of-here maneuver!)
07:27:53 *** HerzogDeXtEr has joined #openttd
08:07:38 <petern> Windows for Arm dev kit?
08:10:34 <pickpacket> petern: is that going to be yet another target platform to compile to?
08:11:23 <petern> Potentally, there are not many devices.
08:13:18 <petern> If they run Windows 11 (rather than 10) they can emulate x86-64.
10:19:49 *** gelignite has joined #openttd
11:18:02 <petern> Because we don't have enough options... Interface Borders ⬇️ Anorexic, Chunky, TTO
11:25:21 <petern> TGP needs erosion, somehow.
11:25:55 *** Kitrana1 has quit IRC (Quit: Leaving.)
11:26:19 <petern> Whatever happened to the deep water patch?
11:26:48 *** Kitrana has joined #openttd
12:01:52 *** wallabra_ has joined #openttd
12:04:03 *** wallabra has quit IRC (Ping timeout: 480 seconds)
12:04:03 *** wallabra_ is now known as wallabra
12:05:39 <petern> When you write `std::min(std::max(std::min()))` because `Clamp()` asserts...
12:10:44 *** Kitrana1 has joined #openttd
12:10:44 *** Kitrana has quit IRC (Read error: Connection reset by peer)
12:15:12 *** Kitrana has joined #openttd
12:18:00 * petern discovers the `<details>` element
12:20:17 *** Kitrana1 has quit IRC (Ping timeout: 480 seconds)
12:37:25 <petern> > The behavior is undefined if the value of lo is greater than hi.
12:38:56 <LordAro> you generally know the ordering of the values you're clamping between
12:39:28 <LordAro> the dangling reference stuff is worse
12:39:30 <petern> In this case, lo is a variable with external dependency, and hi is fixed.
12:41:30 <petern> Hmm, not very external, just the zoom level, so it's limited to 4x normally.
12:44:50 <petern> As the default news font is 18 pixels, 4 * 18 is 72. Which is MAX_FONT_SIZE.
12:46:15 <petern> I guess MAX_FONT_SIZE could just be increased to account for whatever the max zoom level will be.
12:52:34 <petern> Bah, I can't static_assert it because access into _default_font_height is run-time storage. Hmm.
12:55:31 <petern> Ah, changing from `static const int` to `static constexpr int` works. Not sure what impacts that has.
13:15:31 <LordAro> oh, not reading properly
13:29:52 *** ChanServ sets mode: +v tokai
13:36:24 *** tokai|noir has quit IRC (Ping timeout: 480 seconds)
15:35:20 <petern> Hmm, should I upgrade my router firmware...
15:57:00 *** JustANortherner has quit IRC (Quit: User went offline on Discord a while ago)
16:28:09 *** kobol has quit IRC (Remote host closed the connection)
17:16:26 *** Flygon has quit IRC (Quit: A toaster's basically a soldering iron designed to toast bread)
18:05:32 *** Wormnest has joined #openttd
18:16:26 <Bouke> Any idea why the "Refit" button is greyed out for go-to depot order on a Millenium Z1 (refittable)?
18:23:43 <JGR> Refitting in stations has to be specifically enabled on a per engine type basis by the NewGRF
18:24:21 <Bouke> This is the Milennium Z1, a maglev train from the base set.
18:25:44 <JGR> None of the default engines set this as far as I can see
18:28:20 <frosch> go-to-depot or go-to-station order?
18:29:48 <frosch> anyway, it's not disabled for me
18:30:18 <frosch> oh, lol, there are two refit buttons :p
18:30:19 <Bouke> I was trying to understand the translation for STR_REFIT_NEW_CAPACITY_INCOME_FROM_REFIT (`{BLACK}New capacity: {GOLD}{CARGO_LONG}{}{BLACK}Income from refit: {GREEN}{CURRENCY_LONG}`). Is that income generate from the act of refitting? Thus replace expensive wagons for cheaper ones?
18:31:14 <frosch> it's one of the worser guis around 🙂
18:31:35 <JGR> Looks like a prime candidate for a SZSP_NONE
18:33:00 <frosch> also, yes, newgrf can give you money if you remove the gold plating from the wagon
18:33:22 <Bouke> I cannot seem to find this translation. Ah... I don't have such a newgrf. Makes sense.
18:33:31 <frosch> no idea whether any existing newgrf actually uses it though
18:34:04 <frosch> the default refits will only cost 🙂
18:37:42 <andythenorth> JGR: This made sense at the time, but in 2022 it looks a bit silly 😛
18:38:19 <JGR> The problem is that in many GRFs refitting can also change the vehicle length or drastically change the appearance
18:38:27 <andythenorth> “But but but vegicle grf authors must have control of gameplay”
18:38:50 <frosch> hey, adding refit at stations was almost as dramatic as hiding block signals 🙂
18:38:52 <andythenorth> The main reason at the time was drama
18:39:21 <andythenorth> The other reasons are also valid 😛
18:43:50 <andythenorth> Did we ban the related callback? 😛
18:47:09 <petern> I don't think I've ever used refit at stations.
18:47:29 <Bouke> Hmm so but what's the function of the _other_ refit button?
18:52:20 <andythenorth> Refit at stations ftw
18:52:52 <andythenorth> I’m told that the refit button is for liveries 🎄
18:53:54 <petern> (Also HW accel + Vsync gives me a buttery smooth 45 fps... shit)
18:54:04 <andythenorth> Too early for christmas trees?
18:54:28 <frosch> how many christmas cookies did you eat?
18:55:11 <Bouke> Bouke: mistakes were made
19:00:09 <andythenorth> No cookies yet. Think the UK shops will get Halloween done before too much Christmas starts.
19:00:42 <petern> They are the same, so...
19:00:42 <andythenorth> Bouke: There needs to be an emoji for ‘mistakes were made’ 😛
19:02:27 <frosch> andythenorth: here they start with christmas cookies in september. but going by the rule of capitalism: if christmas cookies tasted actually good, they would be on sale all year
19:03:28 <andythenorth> Livery button for Christmas?
19:03:38 <andythenorth> Frosch made room for it
19:03:49 <petern> Ah, but by limiting supply they make us think they are special and thus will buy more.
19:04:18 <frosch> andythenorth: 2tt already allocated it for honking
19:04:26 <petern> Phew, fixed the Online Players window.
19:04:42 <petern> It's clever, but it's all a bit hodge podge, tbh.
19:04:59 <petern> The honking was closed.
19:07:28 <frosch> ah, i see, i like the hidden-hotkey thingie
19:07:53 <frosch> i had some dos software, and some day i hit "z" and it said "congratz, you found the secret key"
19:08:08 <frosch> i was happy all day afterwards 🙂
19:09:17 <andythenorth> frosch: Oh yes 🙂
19:09:27 <andythenorth> Very important feature
19:14:30 <Bouke> ahh I'm so happy with my custom userscript to show an additional translation + more copy buttons
19:17:33 <andythenorth> Can we use z key to change livery?
19:17:35 <frosch> ah yes, eints only shows other languages matching <lang>_*, it does not show matches for *_<country>
19:17:57 <frosch> meh, stupid formatting
19:19:11 <Bouke> ah so there's a feature close to what I'm needing, but eints doesn't know about the familiarity of germanic languages.
19:20:22 <frosch> in line 489 it compares the first two letters of the isocode
19:22:08 <frosch> (ignore the hand written stuff, i just used some screenshot from some bug entry 🙂
19:23:41 <frosch> there was also the suggestion to make the "other displayed languages" configurable, and store it in a cookie or something
19:23:55 <Bouke> Hmm I think we should also show translations for other languages of the same country.
19:24:21 <Bouke> Hmm yeah that suggestion would also work for me
19:25:28 <frosch> i have no idea how helpful other indian languages are
19:25:53 <DorpsGek> - Update: Translations from eints (by translators)
19:25:59 <frosch> showing tamil and hingi when translating marathi
19:26:36 <Bouke> I have no knowledge of those languages, so dunno how relevant that would be.
19:26:46 <frosch> i also never understood bokmal and nynorsk, and whether they would start a bloody fight, if one or the other was shown
19:27:03 <Bouke> Does eints have some per-user storage?
19:27:11 <frosch> so, i don't think the same-country rule is that useful
19:27:26 <frosch> no, eints has no storage except the translations
19:27:37 <frosch> you can tell by the lack of cookie banner 🙂
19:28:20 <Bouke> it keeps history with my username?
19:28:37 <frosch> the history is not really a history
19:28:37 *** supermop_toil has quit IRC (Read error: Connection reset by peer)
19:28:52 <frosch> it's discarded pretty fast
19:29:31 <Bouke> So could we show related languages based on a cookie? And the cookie can be set from the profile page?
19:30:01 <Bouke> E.g. `additional-languages:de_DE,en_GB,fy_NL`
19:30:33 <frosch> yes, possible for sure, just someone has to implement it 🙂
19:31:11 <frosch> eints was not written by tb, so it's kind of hard to start locally for development
19:31:20 <frosch> lots of undocumented magic involved
19:31:47 <Bouke> I'll first get to enjoy the happy feeling from my userscript, maybe after...
19:32:28 <Arastais> ArastaisviaGitHub: wait this was an accident 🤦 aite i gotta fix it
20:11:33 *** nielsm has quit IRC (Ping timeout: 480 seconds)
20:33:18 *** WormnestAndroid has quit IRC (Ping timeout: 480 seconds)
20:33:54 *** WormnestAndroid has joined #openttd
20:41:58 *** WormnestAndroid has quit IRC (Ping timeout: 480 seconds)
20:58:04 *** HerzogDeXtEr has quit IRC (Read error: Connection reset by peer)
21:04:17 *** WormnestAndroid has joined #openttd
21:22:13 *** Wolf01 has quit IRC (Quit: Once again the world is quick to bury me.)
21:33:18 *** sla_ro|master has quit IRC ()
21:45:08 <petern> +static bool ResizeSpriteInScaler(SpriteLoader::Sprite *sprite, ZoomLevel src)
21:58:04 <petern> Pretty sure this is illegal
22:00:25 <petern> Apart from missing the colours sprite property it still just worked o_O
22:03:15 <Eddi|zuHause> chunky bevels but unchunky text?
22:05:03 <petern> It's hq2x (iirc) for all sprites.
22:08:54 <petern> I guess. The first pass is not bad, the second pass is a bit wobbly.
22:10:29 <petern> There's another scaler implementation that is supposed to be good for 2:1 isometric graphics, but I think there's a bug as it's very glitchy.
22:10:44 <LordAro> it's like those experimental shader things that minecraft had a while back
22:11:20 <petern> This code is from a while back, it's a 2014 patch. I'm really surprised I was able to just slot it in.
22:12:14 <petern> I'm pretty sure it was originally older than that though, I have 2011 screenshots with scaling.
22:13:09 <LordAro> that was longer ago than i thought
22:49:32 *** gelignite has quit IRC (Quit: Stay safe!)
23:07:54 <Eddi|zuHause> anyone remembers the old "double mode" from the miniin?
23:08:07 <Eddi|zuHause> that was like 2007?
23:08:34 <glx[d]> I remember syncing miniin, but I never used it
23:09:27 <Eddi|zuHause> i think it at some point got something similar to hq2x
23:11:57 <petern> There was a double size mode in the win32 video driver originally.
23:12:59 <Eddi|zuHause> miniin was around the time between 0.4.5 and 0.5.0
23:13:20 <petern> It was added in 0.2, and removed much later.
23:14:04 <Eddi|zuHause> then the miniin part was the hq scaler
23:14:25 <glx[d]> there even was a shortcut for double size IIRC
23:14:57 <petern> But it was below the surface level, so it would not have been possible to implement hq2x on top of it.
23:16:00 <glx[d]> double size was very basic yes
23:17:30 <petern> I think it was all we ever needed tbh
23:18:31 <petern> Extra zoom hasn't improved the game
23:21:15 <Eddi|zuHause> i used double zoom a lot
23:21:43 <petern> Checking this scaler code, there's a crude bodge to ensure gaps aren't made in ground tiles.
23:25:49 <Eddi|zuHause> i have a miniin build from 2007 that still runs
23:26:16 <petern> Did it ever run properly? 😄
23:26:22 <Eddi|zuHause> but that's from after i switched to linux, so i couldn't have used double mode on that
23:26:35 <Eddi|zuHause> i played loooots of miniin back in those days
23:27:02 <Eddi|zuHause> which had... flaws... but it was better than normal signals
23:27:42 <Eddi|zuHause> i vaguely remember backporting the "bridges over everything" patch back to miniin
23:28:09 <Eddi|zuHause> as it was discontinued after 0.5.0
23:28:23 <Eddi|zuHause> but it finally died on the c++ move
23:34:52 <glx[d]> we helped a lot with map accessor, and GUI rewrites
23:35:18 <glx[d]> basically all massive code changes
23:35:31 <Eddi|zuHause> yeah, that was all around the same time
23:36:55 <glx[d]> and with svn it was not as easy as git rebase
23:41:46 <petern> svn branches were pretty wild 😄
23:44:10 <Eddi|zuHause> i'm not sure i want to dig deeper to find that patch i'm thinking about
continue to next day ⏵
