Skip to content
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

Upgrade Abolished's Third Ability: Bonfire Spring #2658

Merged
merged 13 commits into from
Dec 10, 2024

Conversation

Luffy117
Copy link
Contributor

@Luffy117 Luffy117 commented Dec 9, 2024

This fixes issue #2645

My wallet address is 0x926A237737E3374801e56b059F18AD554CA22228

Copy link

vercel bot commented Dec 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
ancientbeast ✅ Ready (Inspect) Visit Preview Dec 10, 2024 1:07am

@DreadKnight
Copy link
Member

@Luffy117 I've tested this and it's not quite working. Ability seems to have a range of 2 (instead of 3) and it remains like that, no matter what).

@DreadKnight DreadKnight marked this pull request as draft December 9, 2024 22:57
@Luffy117
Copy link
Contributor Author

Luffy117 commented Dec 9, 2024

That's odd because the code is running fine on my end, it's exactly what you'd expect, let me see if I'm committing the wrong code.

@DreadKnight
Copy link
Member

@Luffy117 Works overall pretty well now. I've found a small bug though: if delaying Abolished and still not using the ability within current round, there's no range increase in the next round 🐻

@DreadKnight
Copy link
Member

DreadKnight commented Dec 10, 2024

@Luffy117 Also, it would be good to make little comments on those game.ts additions just for the sake of clearing out some confusion if someone looks into the code and is rather clueless about what's happening 🐻 wish that stuff was more generic and have Abolished.ts customize some class from game.ts rather.

At the end of the turn check if Abolished has used his third skill, if not and the skill has been upgraded he gains extra movement distance.
@Luffy117
Copy link
Contributor Author

Luffy117 commented Dec 10, 2024

@DreadKnight I moved the determination step to the end stage, which should solve the problem, also added comments. Please check it, thx!

@Luffy117 Luffy117 marked this pull request as ready for review December 10, 2024 00:26
src/game.ts Show resolved Hide resolved
@DreadKnight
Copy link
Member

@Luffy117 Needs one more comment imo and should be good for merging after, started a review thingy to point out.
Found some other bug when testing, but seems it's not related to this; seems delaying can be a bit buggy as it's not overly used and not that tested.

@Luffy117
Copy link
Contributor Author

@DreadKnight I added comments to game.ts as well as where I changed it in create.ts and it should work now.

@DreadKnight
Copy link
Member

@Luffy117 Alright, merging. Boosted bounty -> https://www.mintme.com/explorer/tx/0x7ff79983502634864be6564fc8d62ca614eb47728a071e160ba087d66d69e5ca 🪙

Tweaked that comment a bit 🐻

I've created that new issue that I've discovered recently #2659 . Feel free to poke at it or any other one.

@DreadKnight DreadKnight merged commit 6a5fded into FreezingMoon:master Dec 10, 2024
2 checks passed
@DreadKnight
Copy link
Member

This fixes #2645

@Luffy117
Copy link
Contributor Author

Thank you, it was a pleasure working with you.

@DreadKnight
Copy link
Member

@Luffy117 Likewise 👍🏻 Not many people dare to poke at dealing with the unit abilities 🐻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants