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

Bounty Hunter abilities #2149

Merged
merged 8 commits into from
Apr 11, 2023
Merged

Bounty Hunter abilities #2149

merged 8 commits into from
Apr 11, 2023

Conversation

paulosaramp
Copy link
Contributor

@paulosaramp paulosaramp commented Feb 15, 2023

1st ability needs work, 2nd ability completed, 3rd and 4th are placeholders

Progress for #2147

@vercel
Copy link

vercel bot commented Feb 15, 2023

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

Name Status Preview Comments Updated (UTC)
ancientbeast ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2023 1:55pm

@ghost
Copy link

ghost commented Feb 15, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@paulosaramp paulosaramp changed the title BH abilities work BH abilities work for #2147 Feb 15, 2023
@DreadKnight
Copy link
Member

@paulosaramp Alright, will review this again soon and see about merging if all good or provide you with feedback. Hopefully you'll be able to poke at any possible encountered issues if that's the case, we'll have to see pretty soon.

Will turn Bounty Hunter issue into a meta issue, so that each ability can be poked individually. It actually happened like this before for quite a few of the units; whatever it takes to get progress done :)

@DreadKnight DreadKnight changed the title BH abilities work for #2147 Bounty Hunter abilities Apr 5, 2023
@DreadKnight
Copy link
Member

DreadKnight commented Apr 5, 2023

@paulosaramp I've updated original post to be a tad less cryptic and confusing.

I've tested this and it works pretty well. Some feedback:

  • passive should only trigger a message log if enemy in personal space, without the "enemy in" or "no one" extra stuff, as we're trying to keep those logs small; will probably increase the box soon to make this an one liner & some others
  • would be nice if the extra attack when killing target would be performed after a small delay, to be way less confusing
  • comment consistency would be nice (// Kinda like this one), what I mean is space after slashes and capitalized letter

Let me know if you can make the tweaks within 2 weeks or so, otherwise will merge and open new issues. You can also set the unit playable to true, as we have better separated branches now and we'll also have this implemented soon™ #2184

@DreadKnight DreadKnight mentioned this pull request Apr 5, 2023
@paulosaramp
Copy link
Contributor Author

alright i'll see what i can do

@DreadKnight DreadKnight mentioned this pull request Apr 5, 2023
space after // and avoided specific values and names in case of future changes
@DreadKnight
Copy link
Member

Fixed the comments and opened two new issues, #2198 #2199, so that I can merge this.
You're eligible for XatteR bounty and the new issues have bounties as well.

@DreadKnight DreadKnight merged commit 1056917 into FreezingMoon:master Apr 11, 2023
CyberBishop pushed a commit to CyberBishop/AncientBeast that referenced this pull request Apr 20, 2023
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