IRC logs for #openttd on OFTC at 2019-02-07
⏴ go to previous day
00:02:11 *** octernion has joined #openttd
00:03:42 <Samu> shouldn't do any different
00:03:55 <peter1138> Probably but that's not the point.
00:04:12 <peter1138> Then I suggest posting an issue to github.
00:04:16 <peter1138> Following the template.
00:04:28 <peter1138> Then maybe someone else can replicate it.
00:04:33 <peter1138> But I'm going to bed.
00:04:47 <Samu> needs a server and a client
00:05:04 <glx> always check if issue happens in master before reporting :)
00:05:54 <peter1138> Just document your steps.
00:11:11 <Samu> it doesn't always happen
00:12:16 <Samu> i get two crash reports, do I send them in?
00:12:37 <glx> txt may help, dmp is useless
00:13:17 <glx> because dmp requires your exe and pdb as you built yourself
00:13:55 <LordAro> glx: should it not be the same as one that you built on your system?
00:14:01 <LordAro> (assuming clean master, etc etc)
00:14:21 <glx> no msvc can do things differently on each build
00:14:36 <LordAro> that's rather irritating
00:15:02 <glx> but pdb always match the generated exe :)
00:35:53 <Samu> oops, the crash logs are equal :(
00:47:55 *** Gustavo6056 has joined #openttd
00:52:02 *** Gustavo6056 is now known as Gustavo6046
01:00:07 <Samu> for the client crash, I think the problem is here at network_client.cpp line 1176 and 1174 should be switched
01:01:11 <Samu> CloseConnection makes _networking = false, and when the save occurs, it is already saving with no network
01:01:24 <Samu> there is no instance running on the client, only on the server
01:01:50 *** supermop_work has joined #openttd
01:01:52 *** tokai|noir has joined #openttd
01:01:52 *** ChanServ sets mode: +v tokai|noir
01:03:45 *** supermop_work_ has joined #openttd
01:11:25 <Samu> weird, there are two emergency saves
01:12:33 <Samu> CloseConnection does an emergency save
01:12:51 <Samu> then another is done after the error message
01:13:09 <glx> I edited your comment to link to the code
01:14:27 <glx> indeed seems there's too much saving
01:15:18 <Samu> even the error message is repeated
01:17:16 <LordAro> make a note of it in the issue
01:17:29 <LordAro> it's not directly related, but can probably be fixed easily at the same time
01:17:37 <LordAro> not worth an extra issue
01:19:49 <glx> anything useful must go in the issue :)
01:20:12 *** Thedarkb1-T60 has joined #openttd
01:21:27 <LordAro> Samu: please try to write things properly, don't just copy irc logs
01:21:46 <LordAro> there's so much extra crap in that text
01:24:53 <glx> that's the useful info :)
01:27:35 <glx> and maybe other calls should be checked too
01:29:35 <glx> yes it's recent, but rare enough so it was not reported yet
01:36:44 <Samu> no more crash for client
01:36:54 <Samu> this was indeed the problem
01:37:08 <Samu> as for server, gonna investigate now
01:38:30 <DorpsGek_II> [OpenTTD/OpenTTD] nikolas updated pull request #7086: Change #6173: Update SDL driver to use SDL 2.0 https://git.io/fhamZ
01:40:41 *** supermop_work_ has quit IRC
01:40:42 <Samu> company 2 is the exact one i told to reload ai
01:46:18 <Samu> assert(this->suspend < 0);
01:46:35 <Samu> 0 < 0 false, and asserts
01:46:56 <Samu> int suspend; ///< The amount of ticks to suspend this script before it's allowed to continue.
01:47:31 <Samu> it was never suspended, it's not even started
01:47:43 <Samu> it's right about to start a new one
01:48:54 <Samu> it's trying to execute an out of date DoCommand?
01:49:24 <Samu> the script that was removed before the Reload was probably suspended
01:49:49 <Samu> the script that is about to start is fresh
02:25:34 <Samu> both company and instance are already post reload
02:25:58 <glx> yes but it then assumes it's the AI that called the command
02:26:41 <glx> and of course it expect it to be suspended and waiting for the Continue() call
02:26:44 <Samu> it was the one pre-reload
02:27:09 *** Wormnest has joined #openttd
02:27:43 <Samu> what kind of test to put there :|
02:29:46 <glx> try adding || !c->ai_instance->is_started
02:32:28 <glx> is_started means fully initialised
02:33:40 <Samu> can't access is_started anyway
02:38:45 <Samu> it's protected, not sure if it's the same as private
02:39:07 <glx> not accessible from outside in both case
02:40:02 <Samu> bool IsSleeping() { return this->suspend != 0; }
02:40:54 <Samu> if (c == NULL || c->ai_instance == NULL || !c->ai_instance->IsSleeping() ) return;
02:41:21 <glx> should prevent the crash I think
02:45:27 *** Thedarkb-X40 has joined #openttd
02:51:39 <Samu> gonna rebuild again just to make sure
03:00:54 <Samu> assert(this->suspend < 0);
03:01:39 <glx> hmm for network games it should never be >0 IIRC
03:01:58 <Samu> that IsSleeping only checks this->suspend != 0
03:02:27 <glx> yes because for single player suspend is >0 when suspended
03:03:14 <glx> if (this->suspend < -1) this->suspend++; // Multiplayer suspend, increase up to -1.
03:03:14 <glx> if (this->suspend < 0) return; // Multiplayer suspend, wait for Continue().
03:03:14 <glx> if (--this->suspend > 0) return; // Singleplayer suspend, decrease to 0.
03:03:47 <glx> if it's >0 in network, something is wrong
03:04:22 <Samu> the command was queued 2 ticks away?
03:05:06 <glx> or Continue() has been called when suspend was < -1
03:05:07 <Samu> could 250000 #opcodes be part of it?
03:05:26 <Samu> there can be only too many commands queued, right?
03:06:08 <glx> no, once your AI did a DoCommand it will be suspended until it get the result
03:07:20 <Samu> but the command is not yet sent
03:07:56 <Samu> next tick is executed, unless the queue is full or something that it makes it require 2 ticks?
03:08:05 <Samu> i don't know, just trying to figure out
03:08:05 <glx> script_object.cpp:310 the command is "sent"
03:08:36 <glx> line 342 the script is suspended, and will wait for Continue()
03:12:48 <glx> yes but that's working correctly
03:13:13 <Samu> seems to be the new instance starting that's setting it to 1?
03:13:24 <glx> the problem is the new AI yes
03:22:43 <glx> script_instance:192 add assert(!_networking || this->suspend <= 0); same lines 222 and 243
03:25:24 <glx> but I think the AI called Sleep()
03:26:46 <glx> then the command from the old AI returned
03:27:33 <Samu> _networking is undefined :|
03:29:16 <glx> add #include "../network/network.h"
03:30:51 <Samu> assertion failed right away
03:30:52 <glx> I think it may assert even before you restart the AI
03:31:33 <glx> an AI called Sleep() probably
03:32:20 <glx> but that's for constructor and Start
03:32:54 <glx> there's a Sleep(1) in Start() ?
03:34:18 <Samu> heh, i dont even know which ai started, it picked randomly
03:35:22 <glx> yes makes sense the GS use Sleep()
03:35:23 <Samu> i think the GS is NoCarGOal
03:36:32 <glx> but indeed the only reason suspend can be > 0 is Sleep
03:37:03 <glx> and it's usually ok, except in case the AI is restarted with pending commands
03:37:28 <Samu> // Wait for the game to start this.Sleep(1);
03:38:11 <glx> you can remove the asserts and the include
03:41:14 <glx> I think we need to add IsWaiting()
03:42:25 <glx> bool IsWaiting() { return this->suspend < 0; } after IsSleeping() in script_instance.hpp
03:45:24 <glx> we want to care about the command result only if we are waiting for it
03:46:46 <glx> hmm but that could go wrong if we use DoCommand() but receive the previous result
03:49:19 <Samu> can't get an assert anymore
03:49:56 <glx> yes no assert because we check before the call
03:50:47 <Samu> what could go wrong now?
03:51:31 <Samu> what will it do to the result?
03:51:48 <glx> the old AI called a command, is stopped, the new AI call a command, receive result of old AI call
03:52:40 <glx> if the new call happens after the result, no problem because it has been discarded
03:53:21 <Samu> it'd be put in a que first
03:53:44 <glx> yes and results are given in order
03:55:34 <Samu> wasn't the command called Sleep?
03:56:03 <glx> no Sleep() is when the AI decides to sleep
03:56:17 <glx> no need to wait for the server in that case
03:56:44 <glx> indeed Continue() just convert the remaining waiting time in sleep time
03:57:50 <glx> so for multiplayer AI stopping is "wrong"
03:58:53 <Samu> would it mean the old AI could cause the new AI sleep for the time determined by the old ai? that's funny
03:59:42 <glx> the old AI can't do anything to the new AI
04:00:44 <glx> except the possible incorrect command result
04:04:07 <glx> but that won't be easy to prevent
04:08:21 *** HerzogDeXtEr has joined #openttd
04:09:00 <Samu> line 681 of script_instance.cpp ?
04:09:09 <Samu> doesn't look that serious
04:10:19 <Samu> hmm, it sets a bunch of variables
04:10:26 <glx> yeah probably very low probability to happen
04:11:54 <Samu> ScriptObject::IncreaseDoCommandCosts(result.GetCost());
04:12:01 <Samu> this one may be the serious one
04:12:28 <Samu> if i understand what it do, it will subtract money from the new company?
04:12:41 <glx> but that won't be the place to fix the possible issue
04:13:14 <glx> it's more in AI:Stop() side
04:14:14 <Samu> > openttd.exe!SQClass::Mark(SQCollectable * * chain) Line 529 C++
04:14:40 <Samu> the only thing in the call stack
04:15:32 <Samu> void SQClass::Mark(SQCollectable **chain)
04:16:23 <Samu> Unhandled exception at 0x00007FF63B8C42CF in openttd.exe: 0xC0000005: Access violation writing location 0x000000B8988BFFD0. occurred
04:16:44 <Samu> Exception thrown at 0x00007FF63B8C42CF in openttd.exe: 0xC0000005: Access violation writing location 0x000000B8988BFFD0. occurred
04:17:18 <glx> that's the garbage collector
04:28:19 <glx> and it's initiated in only 2 places AI::GameLoop() and Game::GameLoop()
05:01:49 *** Mahjong1 has joined #openttd
07:17:28 *** sla_ro|master has joined #openttd
07:24:59 <DorpsGek_II> [OpenTTD/OpenTTD] PeterN commented on pull request #7086: Change #6173: Update SDL driver to use SDL 2.0 https://git.io/fh9d0
08:11:35 *** andythenorth has joined #openttd
08:27:43 *** andythenorth has joined #openttd
09:06:09 <andythenorth> \o/ Horse compiled
09:06:13 <andythenorth> it was not compiling
09:07:48 <DorpsGek_II> [OpenTTD/OpenTTD] PeterN commented on pull request #7184: Change: Distribute cargo to multiple stations or industries https://git.io/fh9Fl
09:14:21 <andythenorth> I should just build some default offsets into nml PP
09:14:43 <andythenorth> every newgrf has to piss around with the offsets for every vehicle length
09:15:07 <Eddi|zuHause> provide a default template?
09:17:21 <andythenorth> we should only need the template for 8/8
09:17:30 <andythenorth> the rest is deterministic maths :P
09:18:55 <Pikka> if you draw your shortened vehicles at the front of the 8/8 template, they should use the same offsets. Until you want to reverse them, anyway :)
09:29:20 <andythenorth> default tracks are rather too wide for the trains :P
09:30:08 * andythenorth knew this, it's only noticeable when fixing offsets
10:01:01 *** andythenorth has joined #openttd
10:46:09 *** Sularken has joined #openttd
10:50:23 *** andythenorth has joined #openttd
10:51:31 <andythenorth> time for another infosec meeting!
11:06:28 <Eddi|zuHause> dunno what's happening, i started TF, and it freezes every few minutes, while paused
11:06:54 <Eddi|zuHause> just scrolling around the map
11:11:08 <Eddi|zuHause> in perf top i get lots of "ttm_mem_evict_first" "native_queued_spin_lock_slowpath" and "mutex_trylock"
11:19:24 <Eddi|zuHause> can anyone translate that for me? :p
11:39:19 <peter1138> That is about PCI passthrough to a virtualized hardware.
11:40:16 *** andythenorth has joined #openttd
11:40:35 <Eddi|zuHause> so, probably not that related
11:40:48 <Eddi|zuHause> or someone put me in a virtual environment and i didn't notice
11:51:34 *** Speedy` has joined #openttd
12:15:54 *** sla_ro|master has joined #openttd
12:47:18 <Eddi|zuHause> i'm gonna destroy everything now
12:48:56 *** Eddi|zuHause has joined #openttd
12:49:40 <Eddi|zuHause> hm, this definitely did not work
13:26:14 *** Zeentch has joined #openttd
13:26:46 *** m3henry has joined #openttd
13:27:12 <Zeentch> Whee finally some OTTD people :D
13:29:21 <Zeentch> I was wondering, who was it that made the NML Highlighting xml file that's available to download, and do you think that that person could make a YAML 1.2 file for the highligting as well? I use Sublime text for all of my coding and i've recently started to get into NewGRF for OTTD and would be a lot easier with highlighting available as well.
13:33:38 <Eddi|zuHause> i feel like we discussed that before
13:34:01 <Zeentch> First time im here so i've not been a part of that discussion, how did it end?
13:34:25 <Sularken> With the Apocalypse.
13:35:37 <Zeentch> @Eddi|zuHause, thank you so much for the link :D
13:39:15 <andythenorth> someone gonna make me one for BBEdit too? o_O
13:44:01 <Zeentch> can't bbedit use the xml file that is already around?
13:48:13 <Zeentch> unfortunatly i can't test it since i'm not using a mac so it's kinda hard for me to try out, although i do have a mac mini that i need to sort out so might try it at that point.
14:26:26 <Samu> DLG_FLAGS_SEC_CERT_DATE_INVALID
14:27:03 <m3henry> not using letsencrypt?
14:28:06 <m3henry> oh no, it is, but no-one automated its renewal
14:34:12 <Samu> * Check if the instance is expecting an answer from a DoCommand.
14:35:12 *** andythenorth has left #openttd
14:37:30 <Samu> is it normal for the garbage collector to crash?
14:38:33 <Samu> it's one of those rare crashes that occur from time to time
14:42:09 <peter1138> Is it normal to crash... Hmm!
14:43:32 <Samu> if i try to make it crash again, i may not get it in a timely manner
14:48:46 <m3henry> Solution is obvious: Get rid of the GC xD
14:50:11 <peter1138> Just run out memory instead, yes :-)
14:50:54 <milek7> just needs more swap ;d
14:52:02 <Samu> so it ran out of memory?
14:52:09 <Samu> yesterday's night crash?
14:52:44 <Samu> hmm, then how does it crash?
14:53:08 <peter1138> My comment was in response to "get rid of the GC" :)
14:53:12 <peter1138> Nothing to do with your crash.
14:53:23 <Samu> 03:16:23 <Samu> Unhandled exception at 0x00007FF63B8C42CF in openttd.exe: 0xC0000005: Access violation writing location 0x000000B8988BFFD0. occurred 03:16:44 <Samu> Exception thrown at 0x00007FF63B8C42CF in openttd.exe: 0xC0000005: Access violation writing location 0x000000B8988BFFD0. occurred
14:53:31 <Samu> <Samu> > openttd.exe!SQClass::Mark(SQCollectable * * chain) Line 529 C++
14:53:48 <peter1138> Didn't glx already run through it with you?
14:54:40 <Samu> it's initiated in only 2 places, that's all he said
14:54:58 <peter1138> There's nothing wrong with the GC.
14:55:05 <m3henry> Have you tried using unique_ptr instead?
14:55:13 <peter1138> It's the last comment on your bug report.
14:55:30 <m3henry> Or perhaps something is dangling
14:55:44 <peter1138> What happens is the AI got replaced, but it tried to call the callback of the previous AI.
14:56:05 <Samu> ah, that's not the garbage collector crash
14:56:23 <peter1138> Why do you say that?
14:56:49 <Samu> it crashes in a different place
14:57:08 <peter1138> That's how garbage collectors work.
14:57:48 <peter1138> They collect garbage (unused objects) periodically, not constantly, therefore it will happen in a different place.
14:57:51 <Samu> it's separate from that report, unless you mean it's related
14:58:06 <peter1138> It's very likely related, yes.
14:58:20 <Samu> i didn't force a reload ai, it just happened by itself when i was just watching
14:58:31 <peter1138> Could still happen if an AI dies and then another starts up.
14:58:48 <peter1138> I imagine it's "new" because previously AIs would not start immediately, now they can.
14:59:34 <peter1138> CcAI needs to check that the current AI instance is the same AI instance.
15:01:27 <Samu> the reload ai button was there since ever
15:01:47 <Samu> i doubt it's due to them starting immediately
15:02:02 <Samu> let me try crash in 1.8.0
15:04:52 *** octernion has joined #openttd
15:04:54 <Samu> funny, can't make it crash
15:06:56 <peter1138> 13:58 <@peter1138> I imagine it's "new" because previously AIs would not start immediately, now they can.
15:07:12 <peter1138> Feel free to ignore me :-)
15:09:59 <peter1138> Welcome to "unintended consequences".
15:10:02 <Samu> i know that, but still I don't get how it could be related. Reload AI button is per config slot
15:10:13 <peter1138> AIs can go bankrupt as well.
15:10:20 <peter1138> Then they are replaced, by themselves.
15:10:28 <peter1138> Well, not by themselves.
15:10:40 <Samu> have you seen what Reload AI does?
15:11:55 <peter1138> It kills off the AI, and then starts a new one. So?
15:12:59 <Samu> this is really strange for me, I can't see why it's related
15:13:50 <peter1138> Just pressing reload by itself won't cause an issue.
15:14:15 <peter1138> It's if you press reload just as the AI issued a command and would be expecting an answer for.
15:14:56 <peter1138> after 7151, this could also happen if start_date = 0 and a company goes bankrupt while issuing a command.
15:15:10 <peter1138> Which is how it could happen without pressing Reload AI.
15:18:18 <Samu> aha, got a crash before 7151
15:19:04 <peter1138> By pressing reload?
15:19:16 <peter1138> So still the same issue.
15:20:05 <peter1138> Don't focus on 7151, I only mentioned that as a reason why the issue could occur without pressing Reload ID.
15:20:19 <peter1138> I didn't mention it to blame it for causing the crash.
15:20:21 <Samu> but i can't make it happen on 1.8.0
15:20:33 <Samu> something between 1.8.0 and fa53abe8
15:21:59 <Samu> very strange, i'm trying the same steps in 1.8.0, no crash
15:23:08 <Samu> no matter how much I try, no crash, gonna give up
15:24:04 *** Eddi|zuHause has joined #openttd
15:29:11 <Samu> wondering if it's AI Config related
15:29:22 <Samu> im always restarting random ais
15:30:40 <Samu> requires png library all the old library stuff
15:33:57 <Samu> i can use revert commit, i guess
15:38:40 <Samu> stuff that handles ticks?
15:43:48 *** Eddi|zuHause has joined #openttd
15:45:49 <planetmaker> can we have slightly bit of a live feed for each command executed on your console?
15:51:53 <LordAro> planetmaker: people have tried in vain for quite a while to make Samu less verbose
15:56:15 <Samu> the issue is not originated by 7151
16:03:48 <Samu> glx provided fix, prevents the crash on the reload ai, but.. doesn't totally solve the problem from what I can gather, still gonna PR
16:05:26 <DorpsGek_II> [OpenTTD/OpenTTD] SamuXarick opened pull request #7190: Fix #7188: AI instance crash when reloading AI in a server, and an AI… https://git.io/fh9jW
16:10:12 <peter1138> Samu, nobody said the issue originated in 7151
16:17:44 *** supermop_work has joined #openttd
16:19:04 *** supermop_work_ has joined #openttd
16:19:57 <DorpsGek_II> [OpenTTD/OpenTTD] PeterN commented on pull request #7190: Fix #7188: AI instance crash when reloading AI in a server, and an AI… https://git.io/fh9jy
16:27:12 <Samu> is there a unique identifier for instances?
16:27:21 <Samu> start date, start tick, something
16:29:57 <peter1138> I think it needs a network protocol change :/
16:31:20 *** octernion has joined #openttd
16:31:25 <peter1138> Well, probably not, I'm not really thinking about it at the moment.
16:33:34 <Samu> looking at garbage collector dude, for(SQUnsignedInteger i = 0; i < _stack.size(); i++) SQSharedState::MarkObject(_stack[i], chain);
16:33:40 <Samu> stack size = 1024, allocated = 1024
16:34:44 <peter1138> Yes, don't bother looking at the garbage collector.
16:36:00 <Samu> uhm, okay, but it crashed :|
16:36:13 <Samu> I know, it could be related to this reload ai stuff
16:36:52 <peter1138> The GC crashed because it was given bad information.
16:37:01 <peter1138> There's no issue with the GC itself.
16:37:43 <Samu> gonna try make garbage collector run every tick
16:37:56 <Samu> maybe i can find a way to crash it
16:40:55 <peter1138> You can crash it by giving it bad data.
16:41:59 <Samu> dont know which data was given, didn't pay much attention :(
16:42:16 <Samu> it was something in **chain
16:43:07 <peter1138> Seriously Samu, don't bother wasting your time on it.
16:43:42 <Samu> "access violation writting to "
16:44:44 <Samu> void SQVM::Mark(SQCollectable **chain) {
16:45:03 <Samu> which is a weird place for a crash
16:45:52 <Samu> as for what data it contained, I really have no idea
16:45:55 <DorpsGek_II> [OpenTTD/OpenTTD] nikolas commented on pull request #7086: Change #6173: Update SDL driver to use SDL 2.0 https://git.io/fhHet
16:48:17 <nielsm> yeah seriously, don't attempt to debug a virtual machine for a relatively complex language like squirrel, if you couldn't design and implement it yourself from the ground up
16:48:39 *** Wormnest has joined #openttd
16:49:10 <peter1138> Samu, it probably contained data about an AI instance that is no longer valid.
16:51:05 <Samu> that's why i'm forcing garbage collection every tick
16:51:13 <Samu> to see if i can reproduce it
16:51:37 <Samu> smashing that reload ai button
16:56:23 <peter1138> Write an AI that sends a network command and expects a callback every tick.
16:57:29 <Samu> i wonder if it simply ran out of memory
16:57:30 <nielsm> if you can't trigger it repeatedly then it's likely related to memory layout or timing and likely a use-after-free bug
16:58:54 <Samu> hmm, gonna try the opposite
16:59:15 <Samu> instead of every tick, gonna try every year or so
17:01:47 <Samu> if ((AI::frame_counter & (DAY_TICKS * DAYS_IN_YEAR) == 0)) {
17:03:33 <Samu> oh right, the parenthesis in the bad place
17:03:42 <Samu> if ((AI::frame_counter & (DAY_TICKS * DAYS_IN_YEAR)) == 0) {
17:04:12 <nielsm> unless DAY_TICKS * DAYS_IN_YEAR happens to be a nice bitmask like 0x0FFF that's just a bad way of writing "once in a random while"
17:05:21 <nielsm> yes that's a nice bitmask
17:05:57 <nielsm> that works for triggering the condition every time the low byte is zero
17:06:02 <nielsm> i.e. once every 256 frames
17:06:45 <Samu> ah, I'm doing it wrong then
17:08:06 <nielsm> 74*365 == 27010 in decimal, or 0110100110000010 in binary
17:09:55 <Samu> server running, waiting for a crash
17:18:08 *** HerzogDeXtEr has joined #openttd
17:19:06 <DorpsGek_II> [OpenTTD/OpenTTD] glx22 commented on pull request #7190: Fix #7188: AI instance crash when reloading AI in a server, and an AI… https://git.io/fhHvk
17:27:15 <Gabda> does this count as issue necroing?
17:29:55 <glx> well if the issue is not fixed I guess it's ok
17:33:03 <nielsm> it's not like a closed issue means "this must never be fixed"
17:34:13 <Eddi|zuHause> especially not if it's a "andy closed this in a cleanup spree" ticket
17:34:38 <glx> same for stale bot closed issues
17:35:40 <nielsm> only if the discussion in the ticket leads up to a "this is definitely not a bug, actually intended" conclusion :)
17:36:04 *** synchris has joined #openttd
17:39:30 <Samu> can't get any kind of crash, just reloaded ais and yet nothing
17:39:36 <Samu> oh well, time to give up?
17:43:29 <Samu> it's still july 1950, there was no time for any garbage collection call yet
17:50:10 <Samu> why isn't garbage collection called every tick?
17:50:46 <nielsm> because it tends to take a long time and the value of calling it too often is generally low
17:50:59 <glx> because it's useless to call it more often
17:51:42 <glx> it's called every 255 ticks, but only for 1 AI at a time I think
17:52:23 <nielsm> ai frames may not be every tick, depending on difficulty setting
17:52:59 <nielsm> and the counter for ai frames begins when an ai is started, so every ai will have its gc cycle offset
17:53:43 <glx> I think the crash you got was just a bad luck when pressing restart
17:54:34 <Samu> it wasn't a restart, it was a thing on its own
17:55:13 * glx is trying to see if we can use args of the callbacks to check if it was our command
17:56:54 <glx> like on call the AI store cmd, tile, p1 and p2, then compare that when the callback happens
17:59:33 <Samu> studying the results originated from CompanyID cid = (CompanyID)GB(AI::frame_counter, 8, 4);
18:00:17 <Samu> gonna keep looking at it
18:00:28 <nielsm> ...what, why should the AI frame counter determine a company id?
18:00:31 *** Gustavo6046 has joined #openttd
18:01:03 <glx> it's just so we don't GC all AI at the same time
18:01:36 <glx> AI:frame_counter is shared with all AIs
18:02:02 <Samu> 256: cid = 1; 512: cid = 2; 768: cid = 3
18:02:40 <nielsm> yes, that's how numbers work
18:07:37 <Samu> i wonder what happens when it gets to 14
18:08:03 <nielsm> I'm sure the code will tell you
18:09:10 <glx> the code checks it's a valid AI id so nothing bad happens
18:10:21 <Samu> first frame was already = 1
18:10:38 <nielsm> happens at frame 0x0000, 0x1000, 0x2000, etc
18:10:40 <glx> yes but cid will be 0 after 15
18:12:34 <Samu> 15 is not a valid cid though, just pauses a bit longer
18:14:11 <Samu> if (((AI::frame_counter - 1)& 255) == 0) {
18:14:17 <Samu> maybe this? or not necessary?
18:14:29 <Samu> just so it would start at cid = 0
18:14:33 <nielsm> just runs it one frame earlier than it usually would
18:14:47 <nielsm> also why do you think this is a problem?
18:15:08 <Samu> not a problem, but hmm... nevermind, not a problem
18:15:45 <glx> and basically in single player 0 is not an AI anyway ;)
18:15:50 <nielsm> if you want to know when it runs GC, add in a DEBUG() line
18:16:09 <nielsm> if you want to force a gc cycle to run, hack in a console command to force one
18:18:41 <nielsm> and also keep in mind that squirrel itself will probably run a gc cycle on its own after a while
18:19:08 <nielsm> because that's what garbage collected languages like it do
18:21:11 *** supermop_work_ has quit IRC
18:26:37 *** octernion has joined #openttd
18:35:32 *** Progman has joined #openttd
18:35:33 <DorpsGek_II> [OpenTTD/OpenTTD] SamuXarick updated pull request #7190: Fix #7188: AI instance crash when reloading AI in a server, and an AI… https://git.io/fh9jW
18:43:44 <Samu> can't split the function into 2 like you did
18:45:38 <peter1138> glx, with the callbacks, it may be sufficient to check it at the command-level, based on company, rather than delving into the AI?
18:47:11 <glx> when the AI is restarted company is deleted and a new one is created
18:48:27 <glx> the commands know nothing about companies, only the index
18:50:17 <peter1138> Yup, but you can say the same about the AI instance side, too.
18:50:23 <glx> for now I added cmd as callback args
18:50:59 <glx> so I need to store command args when the script calls and check that when handling the callback
18:51:17 <peter1138> But really it shouldn't be getting to the callback.
18:51:46 <glx> once the command is queued I don't know if we can do something
18:51:50 <peter1138> Hmm, I supose it depends if it's the same company but different AI instance, or different company.
18:52:11 <peter1138> I think a command from a company that got removed and then added as new should not get executed.
18:52:54 <peter1138> I can think of a way but it'd be intrusive to the network protocol.
18:53:22 <peter1138> Not that it matters too much :)
18:53:35 <glx> at least the callback can do some safety checks to be sure it was its command
18:54:50 <Samu> give instance a unique name or so
18:55:51 <peter1138> Map company ID to a unique ID within NetworkSendCommand
18:55:59 <peter1138> Then map it back on the recieving end.
18:56:07 <peter1138> If there is not mapping, reject it.
18:56:24 <peter1138> Actually it could still work with a byte value which is just incremented for each new company.
18:56:43 <peter1138> Then a new company with the same company->index would be a different unique ID
18:58:10 <glx> hmm should also work if a company is merged while sending a command and a new company is then created in the same slot
18:58:29 <glx> (very rare case but possible I guess)
18:59:21 <glx> especially if the server is very slow
19:01:48 *** Eddi|zuHause has joined #openttd
19:28:02 *** m3henry has joined #openttd
19:32:07 <DorpsGek_II> [OpenTTD/OpenTTD] Gabda87 opened pull request #7192: Change: making the style of MakeVoid calls uniform https://git.io/fhHUg
19:33:13 <Gabda> is this kind of change is welcomed?
19:33:28 <Gabda> or I shouldn't bother if I see something like this?
19:34:16 <Gabda> (you can drop one "is" from my first question)
19:34:42 <peter1138> That means the function will be called for every iteration of the loop
19:34:53 <peter1138> Best not to make that change.
19:35:58 <peter1138> Although I see it's done elsewhere, so who knows :p
19:36:14 <Gabda> yes, I also saw it elsewhere
19:36:24 <Gabda> and I think the compiler optimizes it
19:36:26 <m3henry> Without -flto that call wouldn't be optimized too
19:36:31 <Gabda> but makes it more readable
19:37:01 <peter1138> m3henry, well it's inline
19:37:08 <Gabda> hmm, so the compiler does not optimize it by default?
19:37:09 <peter1138> static inline I mean.
19:37:41 <m3henry> oh It is in the same TU
19:37:44 <peter1138> Gabda, probably but...
19:38:01 <m3henry> Well then the compiler would be free to optimize it away if it things there's no aliasing
19:38:12 <peter1138> Gabda, even if the function is optimized away, it could still be the difference between a register compare and a memory compare.
19:38:45 <m3henry> I would just store the value in a const variable
19:38:58 <m3henry> Then I don't need to care if the operation is expensive
19:39:45 <peter1138> glx, hmm, trying to work out how to synchronize my unique on new company creation.
19:39:57 *** gelignite has joined #openttd
19:40:15 <Gabda> I think it also can be a readability vs optimizing question
19:40:18 <peter1138> Oh, NetworkServerNewCompany, perhaps.
19:41:04 <m3henry> I would say that storing the value before entering the loop is more readable, because I know that the value does not change
19:41:13 <Gabda> the first gets better a little, the latter might be a little worse
19:42:12 *** andythenorth has joined #openttd
19:42:16 <m3henry> int const maxX = MapMaxX(); is easy to understand the effect
19:42:39 <peter1138> I think we prefer const int :-)
19:43:44 <m3henry> I recently found out that if you read variable declarations backwards, then it reads as it would in english
19:44:03 <Gabda> my main concern was to make them uniform
19:44:12 <m3henry> So west-const became my standard
19:44:34 <Gabda> I do not insist any of the particular style
19:45:59 <m3henry> It can't work for function pointer declarations or array declarations
19:46:15 <m3henry> But it's better than nothing
19:51:21 <DorpsGek_II> [OpenTTD/OpenTTD] SamuXarick updated pull request #7184: Change: Distribute cargo to multiple stations or industries https://git.io/fh9lr
19:57:41 <andythenorth> one day a faster nml
19:58:10 <andythenorth> how about multi-grfs?
19:58:23 <andythenorth> appears as one grf to openttd, but is actually multiple grf files?
19:59:38 <andythenorth> then I could compile, e.g. one grf per vehicle
19:59:50 <DorpsGek_II> [OpenTTD/OpenTTD] Gabda87 updated pull request #7192: Change: making the style of MakeVoid calls uniform https://git.io/fhHUg
20:00:03 <andythenorth> which would be, perhaps 20x faster when a single vehicle changes out of 200
20:00:10 <andythenorth> I'm allowing an order of magnitude for overhead
20:01:03 <Gabda> I inserted your suggestions
20:01:51 *** supermop_work has joined #openttd
20:02:58 *** frosch123 has joined #openttd
20:03:11 *** supermop_work_ has joined #openttd
20:04:32 *** octernio_ has joined #openttd
20:09:14 <nielsm> there is a restart button some people can push
20:12:31 *** Thedarkb-T60 has joined #openttd
20:15:22 <peter1138> Hmm, so client send command, server executes command, and then I guess sends it back to all the clients, right?
20:15:53 <nielsm> server validates command can execute, then executes it itself and tells everyone else to do the same thing at that tick
20:16:02 <nielsm> as far as I understand
20:23:13 <andythenorth> I cut Horse compile time from 1 minute to 15 seconds
20:23:36 <nielsm> by doing something dumb and in retrospect obvious?
20:23:51 <andythenorth> I deleted 200 of the vehicles
20:24:25 <andythenorth> I think I might rework it to be a minimal set, like Pikka's
20:24:31 <andythenorth> I bored of waiting for compiles
20:24:44 <andythenorth> 10 engines, 10 wagons
20:25:12 <andythenorth> or I do separate grfs
20:26:18 <andythenorth> Iron-Horse-Engines.grf
20:26:27 <andythenorth> Iron-Horse-Brake-Vans.grf
20:30:04 <andythenorth> script musa to upload them all
20:30:11 <andythenorth> peter1138: it's slow for multiple reasons
20:30:13 <Samu> this is the part I was unable to fix "It als shows that the latest version of the AI has been loaded instead but it didn't receive the savegame data because it's incompatible even though min version to load is 1"
20:30:46 <andythenorth> it has some kind of massive ID resolution phase, for action 2 IDs etc
20:31:17 <andythenorth> it's single threaded
20:31:21 <andythenorth> there's no partial compiling
20:31:25 <andythenorth> it does have caches
20:32:38 <Samu> help me think. I save my AI which is version 7 but has a min load version of 2
20:32:54 <Samu> if I load it on a computer that only has version 6 of the AI
20:33:26 <Samu> doesn't load the data, because it was saved as version 7?
20:33:41 <Samu> loads the data, because the min load version is 2?
20:35:11 <andythenorth> Horse makefile sample run time is 52s, of that 47s is spent in nmlc
20:35:33 <andythenorth> to compile 270 trains with a crapload of varaction 2
20:37:25 <andythenorth> if I decompile/recompile the grf with grfcodec, the encode is 2 seconds
20:44:26 *** Thedarkb-T60 has joined #openttd
20:50:40 <peter1138> Nice, got a crash on Reload AI...
20:51:38 <Samu> who's a savegame versioning expert?
20:58:30 <DorpsGek> LordAro: Don't ask to ask, just ask
20:58:50 <andythenorth> how will he know who to highlight otherwise :P
20:59:02 <andythenorth> Samu: broadly speaking, there are no experts
20:59:15 <peter1138> So I've queued up a command for a company that no longer exists.
20:59:31 <peter1138> Which means my code is on the right track, BUT, i'm doing the company -> unique_id mapping too late.
20:59:33 <LordAro> that seems inconvenient
21:03:32 <peter1138> I could try checking that the company exists but that kinda defeats the point of the mapping.
21:14:13 <DorpsGek_II> [OpenTTD/OpenTTD] LordAro requested changes for pull request #7192: Change: making the style of MakeVoid calls uniform https://git.io/fhHk4
21:28:16 <LordAro> arbitrary reminder for someone to review ^
21:28:26 <LordAro> i've bumped the date to sunday now :p
21:41:11 <DorpsGek_II> [OpenTTD/OpenTTD] Gabda87 commented on pull request #7192: Change: making the style of MakeVoid calls uniform https://git.io/fhHkQ
21:42:09 <LordAro> Gabda: yeah, like the old commit :)
21:45:11 <Gabda> peter1138 and m3henry suggested that adding the variable might be a good idea, so I created a new version
21:45:53 <LordAro> peter1138: ^ thoughts on #7192?
21:45:54 <Gabda> but as there are different preferences, maybe you should talk about it
21:46:23 <Samu> a version 7 with a min version of 2 can't load on a version 6
21:47:33 <peter1138> I'd imagine because 7 > 6;
21:47:34 <DorpsGek_II> [OpenTTD/OpenTTD] nielsmh updated pull request #6965: Add: Option for population-linear town cargo generation https://git.io/fpkqa
21:48:15 <peter1138> LordAro, I didn't specifically request const, but I did say putting the function call inside the for () condition was maybe not so good.
21:48:30 <DorpsGek_II> [OpenTTD/OpenTTD] nielsmh commented on pull request #6965: Add: Option for population-linear town cargo generation https://git.io/fhHkp
21:51:11 * LordAro sees what godbolt says
21:59:09 <nielsm> modern compilers are pretty good at simplifying expressions also involving many layers of (inline) functions and more
22:00:19 <peter1138> So it already loads it into a local variable.
22:00:22 <peter1138> Er, register, I mean.
22:02:12 <peter1138> I like that when MakeVoid is a NOP it's just... nothign :p
22:03:48 <peter1138> Gabda, so apologies, I was talking bollocks :p
22:04:32 <Gabda> no problem, it wasn't much work, and we learned something in return :)
22:05:37 <Gabda> I'll push the first commit back tomorrow (with the changed commit msg), but I have to go now
22:06:41 <nielsm> okay, I just looked ath that BitpackedUint template thing I wrote the other day again
22:06:51 <nielsm> both gcc and clang optimize it nicely
22:07:08 <nielsm> in fact it's pretty terrible
22:08:49 <peter1138> MSVC is pretty terrible for LordAro's example too.
22:09:38 <LordAro> it's not great, it has to be said
22:09:45 *** octernion has joined #openttd
22:09:51 <LordAro> even when adding /Ob /Og
22:10:26 <peter1138> Seems to be ignoring all the inlines, as it's allowed.
22:11:20 <LordAro> even adding __forceinline does nothing
22:11:57 <LordAro> wait hang on, is it just that it's not optimising the functions away, so it looks bigger?
22:12:42 <nielsm> yeah in your example it's leaving in the functions
22:12:50 <nielsm> but optimizing the actual main()
22:13:05 <LordAro> looks like the loop is still there though
22:13:24 <LordAro> wait, of course it is
22:13:45 <LordAro> yeah, looks much the same as gcc
22:18:27 <LordAro> has it tried to unroll the loop?
22:19:13 <LordAro> no, something to do with the volatile variable
22:19:41 <LordAro> if i change it to an extern variable, it behaves much better
22:29:09 <DorpsGek_II> [OpenTTD/OpenTTD] SamuXarick opened pull request #7193: Fix #6468: Load correct version of AI as specified during the time of its save. https://git.io/fhHI1
22:33:13 <andythenorth> bananas cert outdated?
22:33:49 <LordAro> no? bananas.openttd.org doesn't have https
22:34:34 <LordAro> wait, yes it does, chrome is hiding it from me
22:34:41 <m3henry> Is there pointer aliasing going on?
22:39:08 <dwfreed> andythenorth: "Not Valid After: Wednesday, November 27, 2019 at 18:59:59 Eastern Standard Time"
22:39:17 <dwfreed> cert is valid for *.openttd.org
23:01:48 *** octernion has joined #openttd
23:03:37 <peter1138> White stilton with mango and ginger.
23:03:42 <peter1138> Not really proper cheese, is it?
23:04:36 <peter1138> Oh... there were some builds, apparently.
23:05:14 <peter1138> We should just go with it an add NRT for 1.9 :p
23:05:34 <peter1138> Then a week later release 1.9.1 with some of the bugs fixed.
23:05:56 <peter1138> I'm kinda not joking.
23:05:58 <LordAro> well we've not reached feature freeze yet
23:06:07 <peter1138> We put off massive features and it just gets... never done.
23:08:52 <Samu> there are differences between ai_sl.cpp and game_sl.cpp
23:10:05 <peter1138> It builds, ship it!
23:10:45 <andythenorth> it has been more widely play tested than normal
23:11:00 <andythenorth> there was no NoAI or NoGo support iirc though
23:11:49 <Samu> btw I found a way to detect if a GS is active in scenario editor without resorting to my ai gui stuff
23:12:47 <supermop_work_> peter1138: if you don't put it in 1.9, when will it go in
23:12:47 <Samu> the big Game Script rectangular button is not grayed out if it's active, and it is if it's not
23:12:57 <peter1138> They do build road vehicles at least :p
23:13:05 <andythenorth> it will go in 2.0
23:13:08 <peter1138> Did AIs ever build trams?
23:13:10 <andythenorth> otherwise known as 10.10
23:14:26 <andythenorth> at least nrt has nml support :)
23:14:41 <supermop_work_> i just mean, people have been messing about with NRT since before 1.8
23:14:43 <andythenorth> not merged but eh
23:14:44 <peter1138> Ok, they build tramlines too.
23:14:56 <peter1138> Not efficiently, mind.
23:15:11 <peter1138> No trams on it, haha
23:15:27 <supermop_work_> and it works pretty ok, most of the 'problems' seem to be philosophical differences as to what NRT might do
23:15:55 <supermop_work_> but without it in, idk if you will get a better critical mass of testing than you do now
23:16:30 <supermop_work_> at least, i am certainly not motivated to do more nrt grf work for the past year-ish
23:17:57 <peter1138> Hmm, no notification.
23:18:39 <Samu> loading a scenario is loaded as if it was a savegame
23:18:55 <peter1138> No, they are the same.
23:19:12 <m3henry> What would count as a big enough change to make 2.0?
23:19:46 *** andythenorth is now known as Guest479
23:19:46 *** andythenorth has joined #openttd
23:19:57 <Samu> or maybe the problem is during save
23:19:59 <LordAro> i'm not sure anyone knows
23:20:02 <Samu> have to investigate this
23:20:31 <andythenorth> stick nrt in the RC, revert if fail?
23:20:37 <m3henry> Probably best to go with 1.10 then
23:20:44 <andythenorth> it's not like we force upgrades on people
23:21:22 <LordAro> depends how badly it fails, i imagine
23:22:38 <peter1138> Lots of subtle things are touched.
23:23:43 <peter1138> That took you a while...
23:23:49 <DorpsGek_II> [OpenTTD/OpenTTD] PeterN dismissed a review for pull request #6811: Feature: Add NotRoadTypes (NRT) https://git.io/fh305
23:26:04 <DorpsGek_II> [OpenTTD/OpenTTD] M3Henry commented on issue #7125: Convert singleplayer game to multiplayer server https://git.io/fhHL0
23:27:06 <DorpsGek_II> [OpenTTD/OpenTTD] PeterN commented on issue #7125: Convert singleplayer game to multiplayer server https://git.io/fhHLu
23:27:48 <peter1138> If it add it to beta1, we'd get loads of testing.
23:27:54 <peter1138> And mostly no bug reports, but still.
23:30:32 <peter1138> docs/landscape.html is wrong.
23:31:13 <peter1138> Apparently I never updated the docs when I did the 16 -> 64 change.
23:34:21 <Samu> launching scenario editor does it correctly
23:34:33 <Samu> loading a scenario does it wrong
23:41:36 <m3henry> Can't believe I hadn't thought until now to do a dist upgrade
23:45:47 <Samu> SM_EDITOR versus SM_LOAD_SCENARIO
continue to next day ⏵
