01:56:00 <Gretell> Windows builds of master branch on crawl.develz.org updated to: 0.21-a0-128-gf2b353c
02:53:14 <Gretell> Monster database of master branch on crawl.develz.org updated to: 0.21-a0-128-gf2b353c
03:11:51 <Rotatell> Unstable branch on crawl.beRotato.org updated to: 0.21-a0-128-gf2b353c (34)
13:53:29 -!- yesno_ is now known as yesno
17:00:56 <Floodkiller> gammafunk: any gnoll PR news?
17:43:44 <gammafunk> Floodkiller: well, it takes times to refactor code like this! https://cdn.discordapp.com/attachments/205316046230388737/336261469916233728/unknown.png
17:44:01 <gammafunk> besides, I still have about 8 hours until the weekend is over
17:44:06 <gammafunk> I don't turn into a snozzcucumber before then
17:46:00 <Floodkiller> whoops, that must have been some midnight code. sorry
17:48:29 <gammafunk> Floodkiller: also player-stats.cc:108
17:48:41 <gammafunk> although I'm not sure what that looked like beforehand
17:48:53 <gammafunk> oh, nm
17:48:56 <gammafunk> looks like same type of thing
17:51:14 <gammafunk> yeah there's another instance of this
17:51:39 <|amethyst> hm
17:52:10 <gammafunk> Floodkiller: so I guess in general just remember that early return is better than making a new all-encompassing if statement when there's no alternate conditions to check
17:52:34 <gammafunk> seems to be a thing you've done regularly, stuffing everything into and else {...} when there's no need
17:52:34 <|amethyst> I wouldn't say "better" always, but yeah I'd probably use one here
17:52:51 <|amethyst> because it's already pretty deeply nested
17:53:10 <gammafunk> |amethyst: you wouldn't say it's always better if there's no alternate condition to check and it's a big block?
17:53:28 <Floodkiller> I think I remember fixing it on one and forgetting to go back and fix the rest, but I'm not certain; my fault for missing it though
17:54:08 <gammafunk> I could see maybe when it's one to a few lines in the else, but if it's a big block of code, seems bad to add a level of indent like that
17:54:24 <|amethyst> gammafunk:  I think an if is slightly easier to reason about
17:54:28 <|amethyst> gammafunk:  which would be more relevant if there were more stuff before the if
17:55:47 <gammafunk> I suppose it's easy to forget about that early return
17:55:52 <|amethyst> yeah
17:55:54 <|amethyst> here for example
17:56:04 <|amethyst> I would make the new gnoll check into an early return
17:56:17 <|amethyst> but I would be hesitant about also making the wanderer check into an early return
17:56:47 <gammafunk> which one, the one in jobs.cc that I mentioned?
17:56:50 <|amethyst> because with two early returns at different places in a function it's too easy to miss one
17:57:00 <|amethyst> yeah, the one you pasted a screenshot of
17:57:24 <gammafunk> hrm, but there's only one return in that function
17:57:35 <gammafunk> job_stat_init()?
17:57:56 <|amethyst> I was talking about the left hand side, before your refactor
17:58:03 <|amethyst> where there are 0
17:58:32 <gammafunk> sure, but you said "with two early returns"
17:58:33 <|amethyst> on the RHS, notice how the if (job == JOB_WANDERER) is also an if that governs the entire rest of the function, same as the one you refactored
17:58:48 <|amethyst> so you could also turn that into  if (job != JOB_WANDERER) return;
17:58:51 <gammafunk> oh I see
17:58:54 <gammafunk> yeah if you changed both
17:59:11 <gammafunk> heh, I guess I just like early returns :)
17:59:19 <gammafunk> and dislike a lot of code indent
17:59:45 <|amethyst> I'd consider how to change that inner if
17:59:57 <|amethyst> I guess a random_weighted would be uglier
18:00:10 <|amethyst> OTOH, it would avoid a potential infinite loop
18:00:46 <|amethyst> oh, coinflip
18:01:04 <|amethyst> I guess not infinite loop, but still
18:01:08 <gammafunk> hrm
18:01:14 <gammafunk> let me push something really quick
18:01:20 <|amethyst> I was also thinking about whether it would be good to define a random2<stat_type>
18:01:42 <|amethyst> or maybe more general, a random2 template for any enum
18:02:07 <|amethyst> problem with the latter is that there are things like bitfields where that's not really what you'd want random2 to do
18:02:20 <|amethyst> and it would be better to give an error in that case
18:02:36 <Lantell> Unstable branch on underhound.eu updated to: 0.21-a0-128-gf2b353c (34)
18:03:08 <|amethyst> but I guess a random_choose_weighted would avoid the need for a cast as well
18:04:07 <|amethyst> const auto stat = random_choose_weighted(you.base_stats[STAT_INT] > 17 ? 1 : 2, STAT_INT, /*...*/)
18:04:30 <Cheibriados> New branch created: statlockgnolls (3 commits) 13https://github.com/crawl/crawl/tree/statlockgnolls
18:04:30 <Cheibriados> 03Floodkiller02 {gammafunk} 07[statlockgnolls] * 0.21-a0-129-g0124229: Replace Gnoll aptitude mechanic with locked stats 10(2 months ago, 16 files, 302+ 312-) 13https://github.com/crawl/crawl/commit/0124229c830a
18:04:30 <Cheibriados> 03Floodkiller02 {gammafunk} 07[statlockgnolls] * 0.21-a0-130-gf0fb5d9: Raise Gnoll statline from 5/5/5 to 7/7/7 10(9 weeks ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/f0fb5d998ba9
18:04:30 <Cheibriados> 03Floodkiller02 {gammafunk} 07[statlockgnolls] * 0.21-a0-131-gbd241bf: Mark !degeneration as useless for Gnolls 10(4 weeks ago, 1 file, 2+ 0-) 13https://github.com/crawl/crawl/commit/bd241bfa7187
18:04:43 <gammafunk> just a rebase of the PR
18:11:30 <gammafunk> |amethyst: re the use of indent vs not, in my mind being a gnoll is an exceptional case, so you handle its case in an if rather than handling the usual case in an elf
18:11:47 <gammafunk> and the same logic would allow that if to exist for Wn, since it's exceptional
18:12:12 <gammafunk> but maybe that's not how most programmers think about situations like this
18:13:28 <gammafunk> do you recommend doing that random_choose_weighted(), presumably with each stat explicitely listed like in your example?
18:14:31 <|amethyst> gammafunk:  yes, because it avoids the need to reloop and the i-- stuff
18:15:02 <gammafunk> alright, will do
18:15:10 <|amethyst> gammafunk: could put it into a _pick_random_stat function if it looks too heavy
18:15:25 <|amethyst> or _pick_random_starting_stat or whatever
18:15:31 <gammafunk> the PleasingFungus way, make a new function
18:17:08 <|amethyst> it'll get inlined away anyway, as long as it's defined in the same translation unit
18:33:03 -!- DubDrop is now known as filthy
18:36:54 -!- GiantOwl is now known as Kalir
18:48:47 <travis-ci> The build passed. (statlockgnolls - bd241bf #8530 : Floodkiller): https://travis-ci.org/crawl/crawl/builds/254251708
19:05:20 <Cheibriados> 03gammafunk02 07[statlockgnolls] * 0.21-a0-132-g39ea6bc: Slightly refactor a stat gain function (|amethyst) 10(32 minutes ago, 1 file, 14+ 18-) 13https://github.com/crawl/crawl/commit/39ea6bcd9d0e
19:05:20 <Cheibriados> 03gammafunk02 07[statlockgnolls] * 0.21-a0-133-gbbaa2d2: Remove indent from stat gain notification code 10(32 minutes ago, 1 file, 152+ 157-) 13https://github.com/crawl/crawl/commit/bbaa2d2862db
19:05:35 -!- illusion is now known as illusion-
19:26:14 <|amethyst> gammafunk:  hm, if you squashed that last one into the relevant commits, it would be easier to see what's going on; that's a lot of diff noise
19:26:36 <|amethyst> gammafunk: OTOH, one can just do git diff across the whole branch
19:26:41 <gammafunk> oh, you mean into FK's commit?
19:27:03 <gammafunk> yeah, I'm a little reluctant to do that I guess
19:27:44 <|amethyst> I guess there's also git {diff,log -p} -b
19:28:34 <|amethyst> which seems to work pretty well here, so never mind :)
19:51:12 <gammafunk> Floodkiller: doesn't seem that the commit to add the case for potions of degen is necessary
19:51:16 <gammafunk> it's already covered by is_bad_item
19:51:44 <gammafunk> so I'm going to remove that commit
19:52:05 <Floodkiller> is bad item marks as red, so I added that commit for clarity if the user IDs it
19:52:10 <Floodkiller> but if you don't think it is necessary, feel free
19:52:30 <gammafunk> well, I'm looking at is_useless_item
19:52:45 <gammafunk> that already has a check for is_bad_item
19:53:06 <gammafunk> so I don't see how that case you added is ever reached
19:53:44 <gammafunk> all behaviour should be identical either with or without that commit is what I'm saying
19:54:16 <gammafunk> specifically looking at item-name.cc:3542
19:54:39 <gammafunk> ah, I see
19:55:00 <gammafunk> hrm
19:55:28 <Floodkiller> I mean, it is always bad and I think degen stops you from drinking it if it is ID'd
19:55:35 <Floodkiller> so it probably doesn't matter which way it gets color coded
19:57:50 <checkers> what would a new species be without a few cosmetic special cases
19:58:04 <gammafunk> yeah as far as I can tell
19:58:07 <gammafunk> the commit will do nothing
19:58:21 <gammafunk> since unknown items are not marked as useless
19:58:40 <gammafunk> and if the degen pot is knows, it is always bad, hence already marked useless
19:58:50 <gammafunk> so that case the commit added will never be reached
19:59:02 <gammafunk> so I'll remove that commit
19:59:13 <gammafunk> s/knows/known
20:45:40 <Cheibriados> 03Floodkiller02 {gammafunk} 07* 0.21-a0-129-g0124229: Replace Gnoll aptitude mechanic with locked stats 10(2 months ago, 16 files, 302+ 312-) 13https://github.com/crawl/crawl/commit/0124229c830a
20:45:40 <Cheibriados> 03Floodkiller02 {gammafunk} 07* 0.21-a0-130-gf0fb5d9: Raise Gnoll statline from 5/5/5 to 7/7/7 10(9 weeks ago, 1 file, 1+ 1-) 13https://github.com/crawl/crawl/commit/f0fb5d998ba9
20:45:40 <Cheibriados> 03gammafunk02 07* 0.21-a0-131-g289f66d: Slightly refactor a stat gain function (|amethyst) 10(2 hours ago, 1 file, 14+ 18-) 13https://github.com/crawl/crawl/commit/289f66d320cc
20:45:40 <Cheibriados> 03gammafunk02 07* 0.21-a0-132-g076efaf: Remove indent from stat gain notification code 10(2 hours ago, 1 file, 152+ 157-) 13https://github.com/crawl/crawl/commit/076efafe94d9
20:46:17 <gammafunk> leaving the PR open for now
20:47:55 <gammafunk> Floodkiller: there you go, not it can get more player feedback; just a forewarning that I haven't heard much dev opinion excplicitely in favor of the new species, so we'll have to revisit that at some point down the road and see what people think
20:48:51 <Floodkiller> alright; thanks for merging it!
21:09:30 <Jorgrell> Unstable branch on crawl.jorgrun.rocks updated to: 0.21-a0-132-g076efaf (34)
22:07:35 <Lasty> hilariously, if a polearm has +Rage, tabbing against a target 2 spaces away will give the prompt, "Really attack while wielding the +3 trident "Muckah" {freeze, +Rage Int+3 Dex+3}?"
22:08:25 <Lasty> or maybe that's just while being invisible.
22:08:38 <Lasty> Did I somehow break this when I wrote that patch?
22:09:17 <wheals_> did you use the weapon before this game
22:09:25 <Lasty> yes, but not while invis
22:09:37 <Lasty> it seems like it's happening while invis
22:09:54 <Lasty> yep, I bet it was my patch
22:11:18 <Lasty> because it's in "needs_handle_warning", which is used for attacking and evoking too :p
22:11:47 <gammafunk> !blame2 Lasty
22:11:47 <Sequell> LLLaaassstttyyy
22:12:08 <|amethyst> Lasty:  hm, actually doing the check properly is a bit tricky
22:12:29 <|amethyst> Lasty:  because you want to be sure that this is the only item you're wearing that grants evocable invis
22:12:44 <Lasty> ugh
22:12:57 <Lasty> and unlike the other place we check this, you haven't taken it off yet.
22:13:01 <|amethyst> yeah
22:13:06 <Lasty> booo
22:13:25 <Lasty> well, I'm gonna do the piece of this I can safely do right now
22:13:35 <Lasty> since I'm about to head out for the night
22:14:09 <Lasty> ugh
22:14:32 <Lasty> it's also gonna prompt when wielding a weapon that gives invis while invis
22:14:52 <|amethyst> Lasty:  you can check the oper
22:14:52 <Lasty> until we somehow solve that issue of knowing which item is granting the status
22:15:03 <Lasty> wield and unwield both use OPER_WIELD
22:15:17 <|amethyst> Lasty:  hm, what if you check whether it's currently equipped?
22:15:33 <Lasty> that would be a step in the right direction
22:15:48 <|amethyst> ". . . therefore we can't do that"
22:16:07 <Lasty> haha
22:16:07 <|amethyst> "future devs must suffer as I suffer"
22:16:25 <Lasty> Part of me just wants to remove the warning entirely. :p
22:17:50 <Lasty> is there an easy method for checking if something is equipped?
22:18:14 <Lasty> maybe equipped
22:18:19 <Lasty> er item_is_equipped
22:18:30 <|amethyst> Lasty:  you.scan_artefacts and you.wearing_ego
22:18:51 <|amethyst> you'll need both here, since cloak of invis and the artp are both still things
22:19:28 <|amethyst> Lasty:  oh, hey
22:19:31 <Lasty> that would tell me if there's a source of evokable invis, but not if the source is the thing being operated on
22:19:33 <|amethyst> Lasty:  and that will let you count
22:20:01 <Lasty> oh, do those return a count?
22:20:04 <|amethyst> yeah
22:20:05 <|amethyst> hm
22:20:06 <Lasty> nice
22:20:13 <|amethyst> actor::evokable_invis does that already, but doesn't count
22:20:22 <|amethyst> could change it to return an int (and use + instead of ||)
22:20:39 <|amethyst> the counting lets you know if you are removing your only source
22:20:43 <Lasty> yeah
22:20:53 <Lasty> let's see which dependencies that would break
22:23:16 <|amethyst> you'd need something like   you are invis && you aren't uncancellable invis && you have one source of evokable invis && it's OPER_WIELD && the item is currently equipped && (the item is armour with SPARM_INVISIBILITY || the item is an artefact with ARTP_INVISIBLE)
22:23:28 <Lasty> |amethyst: in c++, is it bad style to have an if statement check an int like it was a boolean? e.g. if (i) {}
22:23:50 <|amethyst> I don't think it's terrible style
22:24:08 <|amethyst> if you're concerned you can use != 0, but I don't think it's worth changing the existing callers
22:24:10 <Lasty> I generally would prefer if (i > 0)
22:24:30 <Lasty> but with strict typing it probably doesn't matter as much
22:24:49 <geekosaur> != is better; negative numbers are a thing
22:25:05 <Lasty> geekosaur: well, it depends on what sorts of data you expect, right? In this case, it's a count
22:25:10 <|amethyst> geekosaur:  in this case, a negative number of invis items equipped probably shouldn't count :)
22:26:29 <|amethyst> clearly need dependent types so "this is a regular int but is never negative" can be expressed in the return type
22:40:04 <Lasty> blarg, this isn't wrapping up fast enough. I'll have to push the fix tomorrow morning.