Skip to content

Conversation

@stavrosfa
Copy link
Contributor

This PR is not meant to be merged right away.

I was working on something that eventually asked the question "is unit X available to this civilization?"
So I figured I will push this small change separately and not bloat the thing I am working on.

Now, I wanted to ask what you think about the implementation @TomWerner @esbudylin @WildWeazel (don't feel obliged to comment or anything, just wanted to make sure that if you want to, you don't miss the PR)

I went with unavailableTo instead of "availableTo" because most of the units are available to all, so, having a ~30 line array in each Prototype in the json seemed a bit too much. The most space in the json file is taken up by units like Kings, and then the unique units and units that can only be produced by a wonder (e.x. Ancient Cavalry).

What do you think about not including the availability at all for these units, and rely on uniqueness and if they can be produced by a wonder? I don't know what the code would look like, but if you agree on this, I can at least try to make it work and have a nice API in place.
This will reduce the size and just the eyesore 30 civ arrays, but it will probably mean that it won't be as clear to someone looking at it to figure out what's going on.

Also, I didn't change the standalone json because I wanted to decide on a format first, and then there is Yegor's PR , which when merged will eliminate the need for that second json.

@WildWeazel
Copy link
Member

Hmm. Not sure yet how I feel about unavailableTo specifically, but in general this is touching on an idea that I've mentioned but still need to write up more clearly, which is having more granular and explicit rules.

With Ancient Cavalry for example, it's not that it isn't available to some civs, but that it's strictly unbuildable. The same goes for Kings, though in some scenarios maybe that's not the intent so it should not be implied by the King flag. My point is that we shouldn't think in terms of how Civ3 rules are represented because they carry too many assumptions. If eg. we decouple "buildable by" and "available to" maybe we could have something like a wonder that produces different units or no units depending on its owner. That's not the best example of where this would be useful, but it's the most relevant.

I have a broader concept of tag-matching for all kinds of applicability/availability type of rules rather than explicit lists of yes/no, which is a bigger architectural discussion for another day.

@stavrosfa
Copy link
Contributor Author

Not sure yet how I feel about unavailableTo

Same, but I lean toward not liking it very much 😄

With Ancient Cavalry for example, it's not that it isn't available to some civs, but that it's strictly unbuildable

Yeah, the civ III specifies only to what civ a units is available (as far as I can see), but there is a flag on our end that specifies if a unit is "unproducible" if no civ can build it (which it's a more friendly API representation of having an array of 32 civs that can't build something).
That's why I said it might be redudant to specify to what civs a unit is unavailable, if it already has the unbuildable flag, but perhaps this is not as clear to an end user, be it a programmer or a modder.

That's not the best example of where this would be useful

Actually I was thinking of the same, so not as irrelevant as you might think.
There are more than one ways we can structure this though. We can perhaps specify what unit a civ can build in the great/small wonder itself for example, and not have this info in the unit.

We can make the format more flexible on our end, for sure, right now I am concerned more with, what is the minimum viable solution that will enable me to use this, but not have the next person rewrite the whole thing because this doesn't work for anything but my current need.

I can dedicate time to make this more to our taste if we have a more fleshed out plan (and doesn't involve much Lua, because I kinda suck at it)

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