Allow spell circles to access player inventory #734
Conversation
vgskye
force-pushed
the
circle-player-inv
branch
from
0cea91b to
2f56641
Compare
|
this code looks basically copy pasted from the player casting env, maybe it should just be abstracted into a shared method somewhere? |
|
it is based off of that, but there are some behavioural changes(e.g. pull mainhand first then offhand, rather than other-hand-first or other-hand-only since spell circles are hands-free) that I'm not sure makes sense to unify into one set of methods |
vgskye
force-pushed
the
circle-player-inv
branch
2 times, most recently
from
923e92c to
2111328
Compare
SamsTheNerd
left a comment
SamsTheNerd
left a comment
There was a problem hiding this comment.
i think pulling it out and unifying still makes the most sense. can have it just take a list of hands or something and that should handle that behavior? That seemed like the main thing, was there another issue with unifying it ?
vgskye
force-pushed
the
circle-player-inv
branch
from
2111328 to
544c570
Compare
SamsTheNerd
left a comment
SamsTheNerd
left a comment
There was a problem hiding this comment.
other than comment I left (which I'm not sure about), code looks fine. Should probably be tested a bit more before merging but I'm not doing that rn
| if (!offhand.isEmpty()) { | ||
| out.add(offhand); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
shouldn't this case check both hands, just prioritizing otherhand. for stuff like self refilling artifacts and whatnot.
There was a problem hiding this comment.
i think having this method take a list of hands to check may be better? that way callers can prioritize whichever hand they want while also being able to decide if they want the other hand used also. then just loop over that list, would probably be neater too.
There was a problem hiding this comment.
not sure that this would ever actually be used in a case like that though
There was a problem hiding this comment.
iiuc this is used for Place Block&co, not Recharge&co? I was just retaining previous behaviour when provided with a casting hand though
…ingEnvironment for use in CircleCastEnv
vgskye
force-pushed
the
circle-player-inv
branch
from
544c570 to
74621d3
Compare
2 participants
fixes #638