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

missile + champion #3

Closed
morteeke opened this issue Jul 26, 2023 · 6 comments · Fixed by #7
Closed

missile + champion #3

morteeke opened this issue Jul 26, 2023 · 6 comments · Fixed by #7
Assignees
Labels
enhancement New feature or request

Comments

@morteeke
Copy link

Hey @brittonhayes, me again.

I was looking inside units.yaml to add some units but it appears that it's missing the 'Champion' explanation and 'Missile Weapons'. Except if I am looking over it ofcourse.

Thanks for you time!

@brittonhayes
Copy link
Owner

brittonhayes commented Jul 26, 2023

Well hello again! I can totally add the champion field. That's no problem.

For the weapons, I opted to do an array of objects where all the weapons (missile and melee) are under the weapons array.

So, two options:

  1. I separate the weapons into missile_weapons and melee_weapons arrays
  2. I add the field weapon_type to each Weapon object in the weapons array. Then add melee or missile into that field for each.

Do you have a preference on one over the other?

@morteeke
Copy link
Author

morteeke commented Jul 26, 2023

I think I would prefer option 1 but both will definitly work.

Great work so far by the way!

Edit: Do I just commit the data I wrote or do I need to fork or something?

@brittonhayes
Copy link
Owner

brittonhayes commented Jul 26, 2023

I'll have to update the model in the API spec since we're changing the object's fields and run task gen to re-generate the warhammer.gen.go code. And I'll have to migrate the existing units.yaml entries to follow the new format. Then you'll be free to add your data to the fixtures file via a fork + pull request. 😄

If you were just adding data, you'd just have to do fork + PR, it's just because we're updating the API spec and models themselves, it's a couple more steps.

I can get all that stuff going in about 30min once I wrap with work for the day.

@brittonhayes
Copy link
Owner

Hey again @morteeke! I've added missile_weapons and melee_weapons to the models and API spec so feel free to try them out or add some data to the fixtures!

I wasn't totally sure if I understood your champions feature request so I left that out of this release temporarily. Where can I find more information about "champion explanation"?

@brittonhayes brittonhayes added the enhancement New feature or request label Jul 27, 2023
@brittonhayes brittonhayes self-assigned this Jul 27, 2023
@morteeke
Copy link
Author

morteeke commented Jul 27, 2023

Hey @brittonhayes, looks perfect!

With champion I mean the regulation about having 1 model in the unit as a champion. That model gets for example an extra attack.

ani

@brittonhayes
Copy link
Owner

Ohhhh now I get what ya mean. Okay I'll take a look at adding support for this when I get home tonight!

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

Successfully merging a pull request may close this issue.

2 participants