New feature: Build limits

[craigbrett17]

Something I would've requested, but I thought it seemed easy enough for me to give it a try myself.

For certain types of units or structures, it would be nice to be able to enforce a build limit per player for this particular type. An example is below:

def archmage class soldier damage 100 damage_radius 1 splash 1 cooldown 5 ... ; other super stats build_limit 1 ; only allowed to build one of these! could be any other positive number

This way, super powerful units can be limited by more than food if desired. The same works for buildings.

def nuclear_missile_silo class building ... build_limit 1

This is useful for hero type units, or very powerful structures. Or if you want to restrict building of unit production buildings to a number fitting of their faction (i.e orcs might have greater numbers so can build more barracks than humans).

These are my first real lines of Python, so I hope I've stuck to the correct coding standards for the project and the language.

Hope this comes in useful!

[soundmud]

Thank you! I won't be able to check it before several weeks. I'll reply as soon as I can. Thanks again.

[felipeveigaramos]

Great idea. I think that things like 'dragons' and 'darkarchers' for instance, or even new units would be better with this.

[soundmud]

I'm back, at last. The changes seem to fit well with the existing code. I have converted some tabs to space, some camelCase names to snake_case names. I will add documentation while I'm at it. The default value of 0 for "no limit" is probably OK, unless 0 could be used to forbid a unit. In this case, None would be a better default value. I'll have to make sure that None is valid in a rules.txt file (probably as a semi-empty line). The "build limit reached" message and the build_limit name might cause confusion with population_limit. "Train" is as appropriate as "build". Maybe the name should be unit_type_limit. Maybe I'm overthinking and the name is good enough, as long as there is some documentation.

[soundmud]

You might have noticed that worldorders.py will require some reorganizing. I have tried to clean it up several times and maybe someday I'll succeed... The fact that BuildOrder doesn't inherit from ProductionOrder is unintuitive. ProductionOrder should be renamed someday, if I find a better name. And the inheriting hierarchy of Orders has probably too many layers. Anyway, I will probably move check_build_limit() to the Player class (in worldplayerbase.py).

[soundmud]

I realize that summoned dragons won't be affected by the code. Maybe I'll add some code to Player.add_unit() or execute_summon().

[soundmud]

Since this feature requires several changes in worldorders.py, it's probably time for me to try a refactoring of this part of the code.

[soundmud]

build_limit will probably be replaced with count_limit. If 3 peasants are building 1 farm, the farm might be counted 3 times.

[soundmud]

Fixing the counting method will make the code easier to read by avoiding the trick mentioned in the comment:

check the build limit only if we haven't already reserved the resources

       # so it only gets checked once per build project

[soundmud]

I am adding tests for this new feature, so it will be easy to improve and refactor reliably. I am wondering if the checking should happen earlier, at the is_allowed() stage, so the order isn't even created. The player would have to be added to the parameters of is_allowed(). Not sure yet.

[soundmud]

According to ComplexOrder.is_allowed(), the player can be retrieved through the unit parameter.

[craigbrett17]

One requirement I should've probably specified is that this shouldn't effect acquiring the unit from another player (capturing/enchanting).

I believe the way it was originally done meant that 3 peasants working on that one structure would only count once. Though I'd imagine your way works better (I don't know as I've not looked at that code since last September and only bits of what you're saying are making sense). I need to refresh myself on that code I think. But if you're going to refactor it anyway, maybe I should steer clear.

[soundmud]

Yes, don't worry about the code for the moment. I have cleaned up the automated tests and I will add tests. I take note that converting should not be affected by count limits.

[soundmud]

Filtering the order at an early stage would require to retrieve the unit_class type. While this isn't impossible, generating the menu happens often and can slow down the game. It's not the first time I try to refactor this part of the code. Maybe I'll be luckier this time.

[soundmud]

The big refactoring might be for another day. I have found that in the case of "build" commands, it is more secure to count only the buildings and the building sites, because two orders can concern the same building. So some "build" orders in excess will be accepted but will fail later, when the building site is created. I have started to add tests about this. Then I'll add code for summoning and for upgrades. And for conversion, which should be unaffected by the limit.

[soundmud]

The unit test about limiting building sites is done. I have added code to check the limit just before a building site is created. The test succeeds. I'll do next: "Upgrade to" will work exactly like "train". "summon" and "raise_dead" will be checked at the Player.lang_add_units() level. I'm wondering about "resurrection", since it could be exploited easily. It will have to be checked too.

[soundmud]

At last I have pushed two commits to the master branch: abd19c06af4b3242a978099dde5c96da9552cf7c and c18404b3e5f387339ba791804a81914efcc3bfa2 . The first commit regroups your proposal, without the translated text. The second commit contains my modifications. I hope you will be OK with that. Your proposal is in accordance with the existing code, but I had to be very cautious since I'm not happy with this part of my code. Comments are welcome, of course. I will close this request after a while, and maybe use this feature to fix, for example, the excessive power of summoned dragons and dark archers.

[soundmud]

I will close this pull request since the modifications have been included. Feel free to comment.