03:34:24 Experimental (bcrawl) branch on underhound.eu updated to: 0.23-a0-5261-gd9800d219b 04:47:39 New branch created: pull/5348 (1 commit) 13https://github.com/crawl/crawl/pull/5348 04:47:39 03Aliscans02 07https://github.com/crawl/crawl/pull/5348 * 0.35-a0-758-gbb4aa87988: Make a note if the a noteworthy skill level is passed through. 10(89 minutes ago, 2 files, 6+ 4-) 13https://github.com/crawl/crawl/commit/bb4aa879883e 08:56:02 Hi. Long time (silent) player here. Please take a look at this small pr from me https://github.com/crawl/crawl/pull/5345 08:57:31 (For Discord side, they've since parted) 10:14:04 03CrawlOdds02 07* 0.35-a0-758-gf74bb9bb89: Fix crash in unrand checks for apostles 10(2 minutes ago, 1 file, 6+ 2-) 13https://github.com/crawl/crawl/commit/f74bb9bb894a 10:17:24 <11Odds> (Really could have sworn I tested generating an unrand with that recent change to make skullcrusher illegal, but apparently I just didn't) 10:25:50 <11Odds> Re: https://github.com/crawl/crawl/pull/5345 - I like the idea, this is consistently useful information when consumable shopping and I frequently have to look at my inventory for it. 10:27:54 <11Odds> If others agree I'll review properly and get it in (only current thought is that I'm not sure I love the colour choice atm, probably it should just match the item) 10:51:44 <04dracoomega> I don't mind what it's trying to do, but the PR itself makes me uncomfortable (partially for reasons noted by Ge0FF earlier: https://ptb.discord.com/channels/735056636644687913/747522859361894521/1529443607285858454 ) The tests seem excessive, and the first seems to misunderstand how Crawl's inventory backend even works (ie: it's impossible to have two different stacks of the same item at once). This same error is effectively 10:51:45 repeated in shop_owned_consumable_count() as well. (I agree that matching the colour of the rest of the line would probably be appropriate, given that the price of the item also does.) 10:52:41 <04dracoomega> One could possibly make a case that it would be useful to do it for wand charges, too, if one is doing this much 10:52:49 <04dracoomega> (And ammo??) 11:04:45 <11Odds> Yeah, I'd probably say it should go on wands and ammo for consistency (and would be useful in those places too). I agree with the concern about the inventory backend misunderstanding, and don't really know how to feel about the other tests as they seem reasonable in a vacuum but clearly way more than the typical testing of this size of feature in our codebase (i.e. none) 11:08:02 I mean, are you saying you'd prefer it not have tests? :P 11:16:14 <11Odds> There's something a little odd about a codebase with super uneven test coverage where some small things have loads of tests while many other things have none. But no, overall I think I'm fine with these tests (except the impossible inventory one), and I suspect actually quite a lot of our tests are somewhat similar in that they are there not because they are testing especially important features but because their author was keen on testing 11:16:15 than the average writer of crawlcode 11:18:15 Yeah I think some people are just of the philosophy of test any code you add /shrug 15:44:53 Unstable branch on underhound.eu updated to: 0.35-a0-758-gf74bb9bb89 (34) 17:46:30 <07wizardike> @Odds this breaks seed stability. The item_attack_skill check used to count the locorbor axe as an unenchanted bardeche and thus return polearms as the skill but now it's result will depend on the players highest skill between axes and polearms. 17:51:17 <07wizardike> I think it is also now impossible for gyre and gimbal to be acquired as the you.hands_reqd check used to count it as a one handed quickblade but now counts it as two handed and as there are no normal two handed short blades it will never upgrade one 17:53:59 <07wizardike> I meant to comment these reasons for not setting the item as an unrand in the code when I refactored it, but it seems I forgot 18:02:39 <04dracoomega> Hmm... does this also mean that axe users are vastly less likely to acquire the lochaber axe compared to polearms users? 18:26:40 <07wizardike> I think so. Although maybe crosstraining helps. It was something that probably should have been fixed 18:27:32 <04dracoomega> (I'll be honest; half the time I forget that it can run entirely off axes skill. It feels like a polearm to me.) 18:32:29 <07wizardike> Flavour was it feels like a polearm to me too. Although its mechanics are between an axe and a polearm 19:33:43 <08nicolae> the intended flavor is that it's both, it was implemented as a modified bardiche because i had to pick something to start with (also the bardiche was what the original lochaber axe was renamed to, so, History) 19:34:29 <08nicolae> also i remember putting a check into the "does this use polearms or axes skill" function that checked whether the player was wielding it or not BUT a certain fungal sort told me that was probably unnecessary 19:34:40 <04dracoomega> No, I understand that. I'm just saying that I somehow regularly forget that it isn't just a polearm ^^; 19:36:09 <04dracoomega> Oh boy. Just striking me now that does this mean it's supposed to get both weapon arts? ^^; 19:36:13 <04dracoomega> At once 19:39:36 <11Odds> Ah great spot, thanks. Will fix. 19:43:34 <11Odds> (I guess apostles with unrands also break seed stability in a sense) 19:44:57 <04dracoomega> No more than ?acquirement does, surely? 19:45:38 <04dracoomega> (But this is one of the reasons that item generation RNG is isolated. There are reliable fallback options for vaults trying to place unrands and it should only affect that one item becoming something else.) 19:46:39 <11Odds> Indeed not 22:35:56 Unstable branch on crawl.develz.org updated to: 0.35-a0-758-gf74bb9bb89 (34) 23:01:29 Windows builds of master branch on crawl.develz.org updated to: 0.35-a0-758-gf74bb9bb89 23:12:32 Unstable branch on cbro.berotato.org updated to: 0.35-a0-758-gf74bb9bb89 (34) 23:56:18 Monster database of master branch on crawl.develz.org updated to: 0.35-a0-758-gf74bb9bb89