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.js -> Bounty-Hunter.ts #2458

Merged
merged 8 commits into from
Aug 1, 2023

Conversation

allmtz
Copy link
Contributor

@allmtz allmtz commented Jul 30, 2023

  • Adjusted various type definitions
  • Typed some function parameters
  • Left comments on some TODOs to investigate once all files in abilities are converted to TS

@vercel
Copy link

vercel bot commented Jul 30, 2023

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

Name Status Preview Updated (UTC)
ancientbeast ✅ Ready (Inspect) Visit Preview Jul 31, 2023 6:48pm

@ghost
Copy link

ghost commented Jul 30, 2023

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

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@andretchen0
Copy link
Contributor

Taking on abilities. That's some courage right there, lol. ;)

Just FYI, about the Hex.createTrap deprecation:

I've been slimming down Hex. It just does too much and keeps too much state. Previously traps had to be created with Hex.createTrap(...), but it's a lot more convenient (I hope) to just follow convention and allow new Trap(...).

If you're going to jump back into Bounty Hunter, you can create a Trap with new Trap(...). Or leave it as is – Hex isn't going anywhere too soon.

@allmtz
Copy link
Contributor Author

allmtz commented Jul 31, 2023

Taking on abilities. That's some courage right there, lol. ;)

I'm hoping this will make it safer/easier to refactor the trigger functions in game.ts. I feel like there's a lot of misdirection in these files and just converting BH already helped clarify a lot of that. That being said, it's definitely not easy 😅.

If you're going to jump back into Bounty Hunter, you can create a Trap with new Trap(...). Or leave it as is – Hex isn't going anywhere too soon.

Got it, added a commit removing the deprecated code.

@allmtz allmtz marked this pull request as ready for review July 31, 2023 18:59
@andretchen0
Copy link
Contributor

I'm hoping this will make it safer/easier to refactor the trigger functions in game.ts.

Yeah, that would definitely make things easier. Abilities could use a complete refactor I believe. Same goes for anything starting with query*.

So, if you have any ideas, or find nice examples from other projects, please post them somewhere.

Got it, added a commit removing the deprecated code.

Great!

Fwiw, when I add @deprecated to the JSDoc, I've been trying to include the alternative in the text that follows. That should show up in the editor – assuming you're using something like VS Code/Codium.

@DreadKnight
Copy link
Member

@allmtz @andretchen0 Note that Bounty Hunter has no traps at all #2147, he's a new wip unit, forked from Swine Thug, so the ultimate hasn't been touched yet, but as things are currently done, some code needs to be there as placeholder.

@DreadKnight DreadKnight merged commit ca62a93 into FreezingMoon:master Aug 1, 2023
3 checks passed
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.

3 participants