00:08:36 <11Odds> Looks to me like attracted, summoned monsters are causing crashes because we don't update map knowledge between seeing them (in _input->update_monsters_in_view) and the player action. Fairly reproducible by tabbing around Jeremiah's butterflies. 00:36:55 <11Odds> (Hope that's not treading on @dracoomega's toes, I thought I'd get it in as it's super simple the crashes are somewhat live) 00:37:11 03CrawlOdds02 07* 0.35-a0-639-g2c5d734d5f: Fix a crash from attracting summons 10(5 minutes ago, 1 file, 1+ 0-) 13https://github.com/crawl/crawl/commit/2c5d734d5f6d 01:16:03 szymekc (L14 DsCK) Crash caused by signal #11: Segmentation fault (Lair:5) 03:26:34 03WizardIke02 07* 0.35-a0-640-g4b5696c81a: Fix opening a sealed door not restoring its tile 10(14 minutes ago, 1 file, 2+ 17-) 13https://github.com/crawl/crawl/commit/4b5696c81af9 03:26:34 03WizardIke02 07* 0.35-a0-641-gf783bab7d3: Fix breaking a sealed door not restoring its tile 10(6 minutes ago, 1 file, 2+ 0-) 13https://github.com/crawl/crawl/commit/f783bab7d3c0 03:33:34 Experimental (bcrawl) branch on underhound.eu updated to: 0.23-a0-5261-gd9800d219b 03:41:52 04Build failed for 08master @ f783bab7 06https://github.com/crawl/crawl/actions/runs/28654566651 03:51:41 <04dracoomega> It's not stepping my toes at all! (I'm impressed it was that simple, actually. Last time I looked into this, I feel I spent like... at least 30 minutes failing to find any way to reproduce it, though I suppose the fact that it crashes now makes it easier to notice that it's happened. I... guess I didn't end up using any summoners? But the concerning thing is that I feel reasonably confident that I triggered it at least once from a 03:51:41 non-summon.) Like, I guess attractitis takes care of itself now, so it doesn't entirely matter how many different underlying ways there might be to end up in that state, but it worries me that there are still possible ways that map_knowledge it out of sync with the real monsters in LoS, which might mess up something else querying it. 04:08:11 <11Odds> Yeah I'm certainly not sure it was only summoners - wouldn't be surprised if e.g. fog clearing and revealing a new monster triggered it 04:08:43 <11Odds> The simple cases where we see a new monster because either the player or monster walks don't hit it 04:13:40 <04dracoomega> I wonder if get_monster_at() should actually double-check here. ...and cause a view update if map_knowledge is out of sync? That feels... like a very unexpected side-effect in that kind of a method, hrmm... 04:14:58 <11Odds> It seems correct to crash if our map_knowledge and the truth are out of sync at a time when the map_knowledge matters 04:15:00 <04dracoomega> Anyway, as I said, it probably can just construct a new monster_info now (known-invis monsters get a generic instance of them constructed instead of the full details - which wasn't true at the start of this project, which was why the lua had to pull from map_knowledge directly) 04:15:18 <04dracoomega> But it would be very good to know when there's an issue, yes 04:15:27 <04dracoomega> Since there are obviously other places also querying this 04:15:39 <11Odds> Like tab crashed the game here, but also the butterflies were just shown on the wrong tiles 04:15:48 <04dracoomega> Yeah 04:16:53 <04dracoomega> (I wonder if, instead of crashing, it should recover via updating the cell and instead print a large error message? Like, it's totally possible to fail gracefully here.) 04:17:11 <04dracoomega> That does rely on someone telling us, but they do often seem to 04:21:39 <07wizardike> I'm a bit worried about relying purely on monster::check_redraw here as it has some limitations (like not redrawing halos and other auras). Its fine if these are out of date during a force more but having them out of date while waiting for player input seems bad. 04:25:16 <07wizardike> We could fix this with a call to viewwindow after update_monsters_in_view in _input but that's a very heavy weight fix 04:29:27 <11Odds> Ah, right. Hmmm. 04:33:24 <04dracoomega> How heavyweight is this really? We already have to call it every single time the input was 'moving the player', and often multiple times during every single turn (for many monster actions, individual steps of beams and animations, etc.). It seems a good principle to enforce the idea that the screen presented to the player at input must be accurate, instead of assuming we have correctly redrawn it every prior time something 04:33:24 requiring this changed, doesn't it? 04:35:02 <07wizardike> We spend about 80% of our CPU time in it currently and there are only about 2 or 3 calls to it each turn 04:35:07 <11Odds> Yeah i don’t have the code in front of me but I was thinking along similar lines that we must be trying to viewwindow just before player input, so would expect this to be moving that draw rather than adding a new one? 04:39:00 <04dracoomega> ...really? Last time I profiled Crawl, it was waaay lower than that. (I wonder if this means I was doing something wrong? >.>) 04:39:43 <04dracoomega> A single projectile being fired can call it like 7 times on its own, though 04:44:14 <07wizardike> I just profiled it again and it was only about 69% when resting. It would probably get a bit lower of there were more monsters on the level to. So not quite as bit as I remembered 04:45:37 <04dracoomega> Looks like it is currently called near the end of world_reacts() (done for every turn where time passes) or at the end of _input() itself if time didn't actually pass. 04:47:10 <04dracoomega> I do wonder what would happen if we pulled those out to earlier in _input() 04:52:36 <07wizardike> We could probably pull the call to it out of _prep_input and move it after update_monsters_in_view then? I am a bit worried that this might be a bit bad during force mores in update_monsters_in_view but it might be fine if not too much has happened after the call to it in world_reacts 04:56:05 <04dracoomega> I mean... it feels like it would be bad if force_mores announcing monsters coming into view didn't reflect the actual positions things were at, if this is the implication. (But monster movements already flag their own positions for redraw anyway, right?) 04:58:14 <07wizardike> Right, most (all?) ways for a monster to enter for will redraw the monster (such as doors opening, the player moving, monsters moving). But there are some things like cloud decay that I'm not sure about 05:06:59 <11Odds> It does feel like the screen should really be up to date both for force_mores and for the player input 05:07:53 <11Odds> I wonder about conditionally doing viewwindow if something actually changed (since it’s very rare) 05:22:39 03noncinque02 {GitHub} 07https://github.com/crawl/crawl/pull/5317 * 0.35-a0-691-gb890930441: Merge branch 'crawl:master' into master 10(28 seconds ago, 0 files, 0+ 0-) 13https://github.com/crawl/crawl/commit/b89093044129 06:01:11 <07wizardike> It looks like maw form mesmerizing you might also be a problem for causing an outdated view on the players turn. It also looks like the player moving and monsters moving trigger there own calls to update_monsters_in_view or equivalent and the call in _input is just to handle miscellaneous ways that monsters can enter view like being summoned 06:23:15 <04dracoomega> Like, regardless of any theoretical performance concerns, it feels like it is just good policy to enforce that - at the time the player has any interaction with the game - the view is 100% correct. And to worry about any possible performance issues after that point. The only time where viewwindow() calls feel like an outsized percentage of time spent is when resting, I think? Because hundreds of turns can pass in a moment. (And 06:23:16 even then, resting is still quick enough in my experience that I'm not convinced this is a problem that requires solving. We're still talking 'a fraction of a second' in most cases.) And if that is actually a concern, I wonder if we can just skip some redraws while resting - if we already have that guarantee that by the time the player is asked anything, the view is definitely correct. 06:29:21 <07wizardike> We can't skip calls to viewwindow while resting as we still need to update map_knowledge for example when fog decays and you see new terrain. It would be nice to separate the drawing out of viewwindow but that is hard to do. 06:31:11 <04dracoomega> (It still kind of horrifies me that explore piety is handled in viewwindow() also >.>) 06:31:57 <04dracoomega> Like, it hasn't seeemed to be buggy, but also it feels like an utterly wrong place for it to be (but we don't really have a better one at the moment that could see) 06:33:30 <04dracoomega> When I was writing lurker code recently, I wanted a place for 'tile that wasn't into view comes into view' and there doesn't seem to be any clean point to handle that sort of thing. (Explore piety is only functional because it is once-per-tile and the arbitrary and unpredictable number of further calls to viewview() won't affect it) 06:44:55 <04dracoomega> We apparently do already skip a lot of the rendering part of viewwindow() while resting with default webtiles settings, as I look closer. (And certainly the non-rendeering stuff must happen, of course) 06:45:40 <04dracoomega> Though not packing all the stuff to render, I think? 06:51:25 <04dracoomega> I don't suppose you know if things like tile_draw_map_cells() are actually vital to call when _viewwindow_should_render() is false? It feels like packing cells we are then going to not actually render is superfluous, considering they will only get packed again the next time viewwindow() is called while _viewwindow_should_render() is true. 07:10:30 <07wizardike> Oh, right we actually do skip most rendering. I think I actually fixed that up for non instant resting to. Calling tile_draw_map_cells actually is needed as it updates the tile cache for all squares in view and what squares are in view can change when resting. Instead, we could keep track of which squares are dirty in the tile cache and then call it for those squares when drawing 07:20:43 <07wizardike> So uncancels don't seem to take any turns. If I ask Gozag for a potion petition, exit the game, reload the game, then choose the potion effects, it doesn't take turns or time 07:21:36 <04dracoomega> Oh no 07:27:39 <07wizardike> It probably wouldn't be hard to make the all take time and give berserk no combat penalties. Actually, it might be a little bit harder than I first thought as you need it to take the right amount of time for the action and it doesn't look like any of them set you.time_taken 07:34:19 <04dracoomega> Well, do any uncancellable things actually take non-standard time? 07:39:54 <07wizardike> I don't think so. Slow scrolls is gone right? 07:42:00 <04dracoomega> Correct 08:29:10 <07wizardike> Actually, it looks like uncancels do take time. They seem to take time and let the monsters act when you are quitting which is actually quite bad for the player 08:31:57 <04dracoomega> So time passes when you quit, rather than after you restart and actually take an action? 08:39:27 <07wizardike> Yes. So your action happens after the monsters act rather than before which is rather bad is this can happen as a result of losing connection to the server 08:40:47 Surely donning/doffing with monsters in view when you choose to continue is uncancellable and takes non-standard time? 08:46:09 <07wizardike> That is uncancellable but it is handled as a delay so its fine. Only things that need user input are handled as uncancels 11:57:00 New branch created: greaterunseen (1 commit) 13https://github.com/crawl/crawl/tree/greaterunseen 11:57:00 03hellmonk02 07[greaterunseen] * 0.35-a0-639-ge64cd3ee6b: Emperor Unseen Horrors, for Abyss 10(4 minutes ago, 10 files, 48+ 2-) 13https://github.com/crawl/crawl/commit/e64cd3ee6b84 11:57:00 New branch created: pull/5327 (1 commit) 13https://github.com/crawl/crawl/pull/5327 11:57:00 Branch pull/5327 updated to be equal with greaterunseen: 13https://github.com/crawl/crawl/pull/5327 12:30:05 03CrawlOdds02 07* 0.35-a0-642-gba01ef36c9: Improve freezing cloud's targeter 10(61 minutes ago, 3 files, 16+ 2-) 13https://github.com/crawl/crawl/commit/ba01ef36c994 12:40:05 <11Odds> It's odd to me that I can order my allies around even when I don't have any, and I think it's confusing when you have non-orderable allies. We should probably just disallow ta and friends in that situation, right? 12:43:55 <11Odds> Relatedly, tentacles aren't actually orderable. They pretend to be, but their code is way too eldritch to respect puny mortal concepts like "targets". I think this is quite flavourful, and we should make it official by marking them as false for this kind of thing, and I guess updating their description 13:22:59 Jannitor (L9 OnCK) Crash caused by signal #11: Segmentation fault (D:7) 13:35:57 Jannitor (L13 OnCK) Crash caused by signal #11: Segmentation fault (Lair:2) 13:37:19 Jannitor (L13 OnCK) Crash caused by signal #11: Segmentation fault (Lair:2) 13:42:33 Jannitor (L14 OnCK) Crash caused by signal #11: Segmentation fault (Lair:4) 13:42:38 <04dracoomega> I hadn't really thought about this before, but it makes sense. (Both parts). 13:44:12 <04dracoomega> Also, for the record, the !you.aware_of(*victim) bit in targeter_cloud::affects_monster is superfluous. This can only be called on monsters the player is aware of in the first place 13:44:24 <04dracoomega> (Like, definitionally this should be true) 13:44:58 <11Odds> Ah, makes sense 13:45:54 <11Odds> (Right I see we do use aware_of in various places in target, but only when checking locations not monsters, which makes sense) 13:48:10 <04dracoomega> Yeah, by the time we're trying to get additional information on the monster's suitability, we've already decided there's something there we can aim at 13:48:52 <04dracoomega> (In practice, 'valid_aim' tends to include a lot of monster property checks for some targeters that must aim at a monster of given properties) 13:49:25 <04dracoomega> But affects_monster() is useful especially for AoE things that can be aimed independently of whether this accomplishes anything 14:09:14 03CrawlOdds02 07* 0.35-a0-643-g0a6f76bff1: Don't allow shouting orders without obedient allies 10(89 minutes ago, 1 file, 15+ 2-) 13https://github.com/crawl/crawl/commit/0a6f76bff169 14:09:14 03CrawlOdds02 07* 0.35-a0-644-g8917bb4c26: Mark tentacles as not following orders 10(75 minutes ago, 1 file, 5+ 0-) 13https://github.com/crawl/crawl/commit/8917bb4c26d7 15:43:11 Unstable branch on underhound.eu updated to: 0.35-a0-644-g8917bb4c26 (34) 22:35:33 Unstable branch on crawl.develz.org updated to: 0.35-a0-644-g8917bb4c26 (34) 22:58:20 Windows builds of master branch on crawl.develz.org updated to: 0.35-a0-644-g8917bb4c26 23:22:04 Unstable branch on cbro.berotato.org updated to: 0.35-a0-644-g8917bb4c26 (34) 23:53:59 03noncinque02 {GitHub} 07https://github.com/crawl/crawl/pull/5317 * 0.35-a0-695-g278ac4801d: Merge branch 'crawl:master' into master 10(15 seconds ago, 0 files, 0+ 0-) 13https://github.com/crawl/crawl/commit/278ac4801dc9 23:55:44 Monster database of master branch on crawl.develz.org updated to: 0.35-a0-644-g8917bb4c26 23:59:19 03noncinque02 {GitHub} 07https://github.com/crawl/crawl/pull/5317 * 0.35-a0-696-ged3cb0499b: Create branches.txt 10(59 seconds ago, 1 file, 322+ 0-) 13https://github.com/crawl/crawl/commit/ed3cb0499b9c