Closed SomeTroglodyte closed 1 year ago
Numbers??
Nice! Is it in-game?
But you should really join the enchanting of world of the Unciv Discord, full of mushrooms, peons and trolls. :trollface: This has been discussed and Yairm stated it was planned (which is a good thing). Mockup by @letstalkaboutdune:
https://github.com/yairm210/Unciv/assets/11946570/f521f04a-da0d-4afa-8b6a-f3a115c8e4ac
Also @letstalkaboutdune did some placeholders of animated attacks (sword slash, bombing, etc) which looked pretty cool to start, and Yairm implemented it: https://yairm210.github.io/Unciv/Modders/Creating-a-custom-tileset/#attack-animations
We won't be getting actual Unit art small_airplane for air units anytime soon, right? Animating one in a nice "stuka run" curve might be macabre ghost phun...
Nope, but Dune (again) suggested this: https://github.com/yairm210/Unciv/issues/7482
These little mosquitos flying as they perform an attack or Air Sweep would be nice, though surely a hell to implement.
Nice! Is it in-game?
Of course. I do "let's see how difficult and/or cleanly this can be done" in code, doing so in gimp wouldn't make much sense... https://github.com/SomeTroglodyte/Unciv/tree/AnimateDamage Trigger was - I knew the tooltips can do what this needs, and I was curious whether actually using+tweaking one would go well - nope, copypasta and trimming looks like cleaner code.
Not really happy with the impression. Oversized was intentional to be able to better see the scaling and interpolation effects, but tweaking that doesn't seem enough. They'd need shadows and different trajectories for aggressor and victim... Which I'm sure I could do but haven't got the patience right now.
Mockup by
Ah I see, yes, I forgot the minus signs. That's a pretty simple animation, linear movement no scaling late fade-out, my branch is already a bit more complex but quite close.
animated attacks
Yup, I did see the implementation, but no data (images) to support it - or test. And the wiki entry needs a hand - afaics animations play for the attacker, not the defender and "take damage" doesn't clarify that, plus "Seige" is not quite fitting, sounds like a pastry, while I'm almost sure "siege" is meant. Or maybe "beignet"? Ooooh, that little bakery in 09140 Seix...
Discord
They blocked users of privacy tools so long that now that they stopped doing so I no longer have the patience to learn how to deal with what looks like pure eyestrain chaos. UI gives me headaches and makes me glad I never suffered from epilepsy. Plus, slow as hell - a javascript nightmare - and the desktop app still classifies as malware - only malware tries to install new protocol handlers for usecases that can be done without. Nah, too little benefit for too steep a cost. Ah yes I see you agree, "shrooooooommms" indeed. And not the nice Lewis Carroll type either.
surely a hell to implement
Nah. Elliptical path, rotation = tangent, translate from a FloatAction linear value, done. Maybe add a scale w/ fast-in-slow-fast-out interpolation for takeoff/landing illusion. Syncing with a sound - now that may be trickier.
@SomeTroglodyte A. The numbers you implemented look good - remove the horizontal movement, make interpolation linear, and make the numbers redder, and I think it's ready :D B. "No animation for attacker sprite if air unit" makes sense to me C. MapTo - probably no reason, doesn't look like my code;)
ready
Not at all - that's still just the parameter battleAnimation is getting, and that's not the actual damage, that's the average of the predicted min and max. So, needs work to find when and where the actual number is known and pass the value around.
And that quest has ramifications - see #5259
Plus, slow as hell - a javascript nightmare - and the desktop app still classifies as malware - only malware tries to install new protocol handlers for usecases that can be done without. Nah, too little benefit for too steep a cost.
Yeah, I heard you. I only use the website version (both on PC and Android - on which it is a broken nightmare), never installed the app. For my part, I'm only on Discord for the few open-source projects I follow or I contribute too.
@yairm210 - polishing that code also raises the issue that right-click attacks up till now don't do the animations, and WorldMapHolder.onTileRightClicked in its attack branch vs. BattleTable.onAttackButtonClicked do mostly similar things but there's too many differences to simply merge the code. Some of those stem from bugfixes, so it's sensitive. Others may mean undiscovered bug - like r-click not testing the movePreparingAttack result my bad - it does test it but differently. Then there is the ... how to call it politely ... the AttackableTile / defender issue (AttackableTile.combatant is filled but never used, instead getMapCombatantOfTile is called again - feels a little off; and the bad overhead/gain ratio where getAttackableEnemies is called then filtered for the already known tile (r-click)...
Any input/ideas what/how we can safely consolidate all that so r-click also calls battleAnimation?
Sounds like a function on WorldMapHolder to me
Oh, and the third question up there is still unanswered - I won't simply guess, so either I get some actual unit-attack animation images to test and confirm, or the author confirms that the first path should have used the baseunit name not the type name...
Progress report... Making "Scarlet" redder prompted me to add a slight black shadow since there's not much leeway from ff341c to ff0000, and it also improves readability on tilesets with garish colours.
There's another slight problem - if A kills B while also taking damage, the animation plays with A already on the tile where B was, and both Numbers play on top of one another over tile B... Not sure what to do about that.
Also: Should settings.continuousRendering
have an effect on these...
Since the numbers face out anyway, there's no reason to make them smaller Just make them go up and fade out :)
Also, looks much better than before!
no reason to make them smaller
But - Star Warts!
I agree with Yairm. Making them smaller leaves less time to read them. Otherwise, it looks good!
I agree with Yairm. Making them smaller leaves less time to read them. Otherwise, it looks good!
OK, I was testing with a much longer duration to check geometry, so I didn't notice that aspect. Probably means final duration should be slightly over a second, not 0.8s as I planned. The current animations play a total of 0.7s unless a mod provides more than 4 frames for the sprite animation, with the damage numbers starting in at 0.3s when the attacker starts moving back (by the way, that's also when the sprite animation is started, which feels wrong)...
Discovered two new questions:
Battle.takeDamage()
below the comment //melee attack is complicated
... Can that be fixed or will that have to remain a lie...?A. That's such an edge-case that I think it's fine, in those cases the big question is usually "is this a sure win, sure loss, or up to chance" B. Not intentional, you can add in some other factor if you want (unit owner? Unit type? Whatever you want l
such an edge-case
True, but if a blind :see_no_evil: moron geriatric liablity like me notices, others will too, and that requires at least a little consideration whether a better prediction is possible with a few lines of code or would require some major refactor - My impression is the latter, but others may see an easy way perhaps...
other factor
Base defender value on defender instead of attacker then... :owl:
And last animation, can see enemy movement.
https://user-images.githubusercontent.com/63000004/236915684-9905d1f5-d448-47f0-8334-3afb7e3e8fce.mp4
Did that Carrier just attack? Nope, the Bomber on it did. Which is also why its HP doesn't go down despite the red flash.
Questions...
BattleTableHelpers.WorldScreen.battleAnimation.getMapActorsForCombatant
changing line 100/101 to} else if (!combatant.isAirUnit()) {
would fix that - but is that the right approach?val slot = if (combatant.isCivilian()) 0 else 1
- class TileLayerUnitArt makes no guarantee which slot is used for what -> fragile code merits at least a few comments?private fun getAttackAnimationLocation
-unitSpecificAttackAnimationLocation
andunitTypeAttackAnimationLocation
use the exact same formula. Is the first one meant to useattacker.unit.name
instead ofattacker.getUnitType().name
?}.mapTo(arrayListOf()) { it to it.color.cpy() }.toMap()
and not}.associateWith { it.color.cpy() }
?