AD_1296: Removing previous change and adding support for parameter ta…

[ShaneWalsh]

AD_1296: Removing previous change and adding support for parameter targeting highlighting. By default now all query hexes always highlights the selected hexs by default. Only creature movement sets it to false, so you get the dark path. Selecting a choice path, like Grumble's Boom Box will not highlight the entire path as specified in 1296, and so should any other ability using the same targeting method queryChoice.

updateDisplay was being abused, and was being called everywhere and now it should only be called by queryHexes() and die(). It shouldn't be abused as it was, it made it very hard to keep the UI in a consistent state. Hexegons should be controlling cleaning the display. Now whenever a hex needs to be changed, cleanHex should be called instead to clean only that hex and then apply the changes directly to it, instead of clearing the entire screen.

I think this should resolve #1296 #1295 #1294 and some other stuff.

I have changed alot of code here, especially how things work by default. But its in an effort to get some more consistency across how hexagons works. It will probably break a few abilities, let me know if it does, and why it does and I can try and change them.

If your pull request tries to address a specific issue from the repository, please reference to it.
Don't forget to include a description of the changes proposes. https://discord.me/AncientBeast

[DreadKnight]

Any punctuation required after this?

[ShaneWalsh]

ES6 does not require punctuation, but in order for it to be backwards compatible with older browsers without transpiling it should have one. So I will add it in my next few changes.

[ktiedt]

To clarify, Javascript inherently supports no trailing semi-colons through ASI (Automatic Semi-colon Insertion) and this has nothing to do with old browsers (it is part of the original code base).

However, always using semi-colons is a good thing, because if files are concatenated, you never know the state of other libs that might get concatenated with your code. This is why in the mid 2000's and sometimes still today you can find open source libs that prefix all JS files with a leading semi-colon for no apparent reason... It's to fix the lack of semi-colon when concatenating multiple JS sources. Trailing semi-colons are a good practice and should not be neglected ;)

[DreadKnight]

@ShaneWalsh Nice work so far! Definitely more consistent, still need to do more testing.
One regression so far, the cancel hexagon is missing when hovering non viable targets, #847

[DreadKnight]

A small visual issue here: it would be ideal if the hexagon under red player's Swine Thug would be rendered above the blue outlined ones so that it wouldn't get a bit overlapped at the bottom side.

[DreadKnight]

The hexagons under active medium and large units tend to glitch a bit if hovering on the units themselves while no ability is selected.

Scavenger's Escort Service is a bit glitchy when targeting, as it should display both the Scavenger's hexagons and the target's when picking a new location.

Abolished's Greater Pyre should highlight all nearby hexagons as it's an AoE ability (old issue) #882

[ShaneWalsh]

I pushed changes that should resolve #882 and #847 .

Can you clarify "The hexagons under active medium and large units tend to glitch a bit if hovering on the units themselves while no ability is selected." Is this not just the new position of the unit being indicated by flashing? because when they are out of moves there is no change in the hex on hovering. Can you give me a screenshot of what you want to change, and what you want it to do instead :D

In the mean time I will look at adding the missed punctuation and ill look at the Scavenger's escort.

[ShaneWalsh]

Added fix for Scavenger's Escort Service. It should now highlight all of the final position hexes. Added punctuation.

[DreadKnight]

Tested, nice work! Regarding remaining issue: when moving the unit, the initial location hexagons under the unit shouldn't get overlapped by the "walk path" ones (except in the case of Scavenger's Escort Service, since that's an ability and that kind of overlap behaviour is actually required over there anyway, just saying).

[ShaneWalsh]

Ok so last part of this change, (minus the hex overlap with the wrong color, that's minor and can be done later) is to make the movement path not overlap on the creatures base hex's. However when using a abilities it should, where applicable. I may not get to this tonight, life's a bit hecktick at the moment, but I just wanted to clarify anyway.

[ShaneWalsh]

Ok I added another parameter to tracePosition to allow it to draw over the creatures tile( true by default because thats how it functioned before). When called from tracePath it is always false, so when moving a creature it should not draw the new position over the creatures base hex. Abilities that use tracePosition like scavengers escort should be unaffect because of the default value and so should function the same as before. I tested both uncle fungus and scavenger.

edit* Escort Service does not use tracePath, its a custom fnonselect, so unaffected the change.

[DreadKnight]

@ShaneWalsh tested, moving medium/large units it's fine, but Scavenger's Escort Service targeting is rather problematic, as it should show colored outlines under the whole nearby viable target and when hovering around, should show filled hexagons for the target's new position, plus I don't see the proper overlap, as the hexagons for the new locations should replace the hexagons for current ones on hover.

[ShaneWalsh]

sorry to be a pain, but could you try and explain exactly how you would like the escort ability to work again, but maybe in bullet points. I understand that you want the target that's being moved to have a hex outline(not filled) under it once the ability is activated, thats easy to do.

The rest is a bit difficult to understand.
"when hovering around, should show filled hexagons for the target's new position," It does this already doesn't it? the new positions of both creatures are filled and flashing.

"plus I don't see the proper overlap, as the hexagons for the new locations should replace the hexagons for current ones on hover." I have no idea what you mean by this :D

[DreadKnight]

Basically, when targeting Escort Service, the hexagons for the target new location can overlap (replace actually) the hexagons from under Scavenger itself and the unit that's being moved. More clear now? xD

[ShaneWalsh]

For someone as dim as me you can never be clear enough ;) I think I get the gist now. I think this will require a custom solution for the Escort Service ability.

[DreadKnight]

Ok, small update on the Escort Service targeting, kinda like a train on railroad:

• the possible target path should be done using colored outline hexagons
• when hovering a viable location, filled colored accordingly hexagons representing both units will be moved around
• while doing so, there won't be any duplicated hexagons displayed under Scavenger / unit
• hovering out the viable path, filled colored hexagons should be displayed under Scavenger itself and colored outlines under the other unit

[DreadKnight]

Should be most intuitive this way and avoid visual cluttering and distractions, which is what we're aiming for.

[ShaneWalsh]

I couldn't see a way to do the escort ability that was inline with the default logic so i had to add a new parameter to the queryHexes function. Can now pass in a function to be executed after the queryHexe's logic has executed. In this case we want to draw a outline around the target. So the logic is now passed in from the escort ability and executed by queryHexes.

[DreadKnight]

Good job, I've tested and it's pretty intuitive this time. Only one thing kinda bugs me about it regarding medium and large units as the opposite hexagon is not really usable (drops the avatar from queue), so perhaps differentiating that one with a dashed outline hexagon might be best for now provided it's easy.

[ShaneWalsh]

Hexes on the target creatures should now be dashed, except when they are on the movement path(for medium and large creatures). Let me know if you see anything else, ill check back tomorrow. I'm gonna crash for the night

[DreadKnight]

Not what I was thinking but it's probably better, should do, at least for now. Good job and sleep well!

[DreadKnight]

This also fixed #882