-
-
Notifications
You must be signed in to change notification settings - Fork 600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Godlet Dashboard Toggle #1237
Godlet Dashboard Toggle #1237
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice to see a PR. Will comment more after testing.
src/ui/interface.js
Outdated
const typeMap = activePlayer.availableCreatures.filter(el => el.length); | ||
const deadOrSummonedTypes = activePlayer.creatures.map(creature => creature.type) | ||
const availableTypes = typeMap.filter(el => !deadOrSummonedTypes.includes(el)) | ||
// optional: randomize array, grab a new creature every toggle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalized letters for comments, for style consistency.
src/ui/interface.js
Outdated
availableTypes[i] = availableTypes[j]; | ||
availableTypes[j] = temp; | ||
} | ||
// grab the first creature we can afford (if none, default to priest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalized...
src/ui/interface.js
Outdated
|
||
godletToggle: function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toggleGodlet
would probably be a better function name, hmm...
I haven't tested the patch yet, but as far as I can tell, this might have been approached better as simply having a random bolean parameter for toggleDash
function, because there are quite a few other UI fixes/patches that rely on toggleDash function itself. Gonna comment on the PR after testing anyway.
I've tested this, as as I was saying, because of the function name change for the Godlet Printer ability, the game ends up being rather quirky, best approach would be to keep old function name and have a boolean parameter for random unit selection in order to avoid having to figure out all the issues we dealt with before. |
Aw dangit, didn't want to bloat the other function but neglected performance quirks-- refactored. |
@sammorrow it still introduces that bug when activating the ability and then canceling it, if you notice... |
Altered a condition in closeDash, querymove is now triggered on dash close when activeCreature, not selectedCreature, is a Dark Priest. |
@sammorrow This also broke the recent fix for #1208 for some reason... |
Also, selecting a unit for materialization doesn't indicate that Godlet Printer ability is actually selected... |
Also, if materializing a unit within active Dark Priest's movement range, the range doesn't get updated. |
Regarding my last comment, seems that issue was around but only been made visible because of another PR we're experimenting with atm, so I'm going to open up some new issues related to that as required :) |
I've reverted this for now, feel free to open a new PR when it's a bit less problematic and I'll test it again. |
@DreadKnight Ah, sorry bout the trouble. I've reverted back to the most recent master myself to get a clean PR going, but I'm running into some odd behavior that differs from the live site. With the current master code, if I open the scanner menu, select a creature, then close the menu, I run into the 'uncancelled closeDash' bug (you have to type R to break out of your action and R again to re-open the menu). I checked the code on the live site--closeDash is slightly different there. It's being fed a boolean ('materialize'), it doesn't run into that bug, and it doesn't generate extra 'cancelled' messages when you click the 'materialize' button. Where'd that version of closeDash go? |
@sammorrow No worries. The live version is considered as The |
Referencing #1147
Adds new function to the UI class, godletToggle. godletToggle behaves similarly to toggleDash but calls showCreature with a random, available, affordable creature.
The Dark Priest's Godlet Scanner now calls godletToggle instead of toggleDash.