Author Topic: [DONE][Suggestion] requiresBuyCountry for soldiers (and craft)  (Read 2864 times)

Offline bulletdesigner

  • Commander
  • *****
  • Posts: 685
    • View Profile
Basically just introduce the requiresBuyCountry for soldiers as you got for item's. (isn't listed in the ruleset reference if it already exist)
« Last Edit: April 09, 2023, 10:40:12 am by Meridian »

Offline MaxMahem

  • Captain
  • ***
  • Posts: 60
    • View Profile
Re: [Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #1 on: March 18, 2023, 11:12:55 am »
The feature seemed easy and reasonable, so I went ahead and implemented it. Pull request here. I also added the feature to craft as well. Now the only items lacking it are scientists and engineers. I would have added it to them, but the location to put the key in the rule is not so straightforward, so I held off.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9084
    • View Profile
Re: [Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #2 on: March 18, 2023, 11:58:08 am »
I'll just repeat that there should be a discussion before any PR.

Just because something is easy, doesn't mean we want it.
And "reasonable" is very subjective as we all know.

Don't be surprised and/or disappointed if undiscussed PRs are simply closed without any explanation.

Offline MaxMahem

  • Captain
  • ***
  • Posts: 60
    • View Profile
Re: [Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #3 on: March 18, 2023, 01:38:18 pm »
Sorry, I didn't mean to step on any toes. I do this mostly for the fun of it, so any request I send is either going to be something I want for myself or something that looks fun (and probably easy) to do. If you don't want the patch, feel free to close it. I've already got my enjoyment out of doing it :P

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9084
    • View Profile
Re: [Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #4 on: March 18, 2023, 05:20:41 pm »
I'm just preventively informing... there are (quite a few) people who get mortally offended if their unannounced PR is closed.
Regardless of how many disclaimers I put out there.

The feature is indeed reasonable and bulletdesigner certainly is one of the people I enjoy helping, so it has a good chance of being implemented.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9084
    • View Profile
Re: [Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #5 on: April 08, 2023, 01:38:51 pm »
The feature seemed easy and reasonable, so I went ahead and implemented it. Pull request here. I also added the feature to craft as well. Now the only items lacking it are scientists and engineers. I would have added it to them, but the location to put the key in the rule is not so straightforward, so I held off.

Review on github.

Some formatting nitpicking, one missing feature (stats for nerds for craft) and one bug (buying items).

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9084
    • View Profile
Re: [DONE][Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #6 on: April 09, 2023, 10:40:38 am »
Fixed and merged.

Offline bulletdesigner

  • Commander
  • *****
  • Posts: 685
    • View Profile
Re: [DONE][Suggestion] requiresBuyCountry for soldiers (and craft)
« Reply #7 on: April 09, 2023, 04:29:01 pm »
Nice thks