OpenBW / BWAPI4J

BWAPI wrapper for Java
GNU Lesser General Public License v3.0
24 stars 9 forks source link

Zerg ResourceDepots refactor/training functionality, MobileUnit build time, BWEM initalization #63

Open kdlin10 opened 6 years ago

kdlin10 commented 6 years ago

Hatchery, Lair, and Hive now inherit from an abstract ZergResourceDepotImpl and can train units. (I know some people dislike exploding number of classes, but thankfully the game isn't going to change much so the scope for growth is limited). Player class canMake() returns true if checking a Hatchery can train something that morphs from a larva (Does not check amount of larva).

MobileUnit can be queried for their remaining build time; returns "Number of frames remaining until the current training unit becomes completed, or the number of frames remaining until the next larva spawns."

BWEM now assigns starting locations as part of initialization

Bytekeeper commented 6 years ago

As you mentioned I'm not really happy about this approach: This is the first of many many interfaces: TerranResourceDepot, ZergFlyer, ProtossSpellCaster, ZergSpellCaster, TerranSpellCaster. Since Hive extends Lair extends Hatchery - you can already just do a instanceof Hatchery. What does the new interface improve?

(canMake is another issue (#53) and is ambiguous anyways.)

adakitesystems commented 6 years ago

I agree that TerranResourceDepot, ZergFlyer, ProtossSpellCaster etc. could be too much. However, since all of the Zerg resource depots are almost the same, I like the ZergResourceDepot. I also agree instanceof Hatchery works well too but I would argue this check is slightly disingenuous if the user merely wants to check if the building/unit is resource depot. I, personally, consider it a hackish type of check. The code instanceof Hatchery insinuates the author wants to check for Hatchery but the actual underlying purpose of instanceof Hatchery could be to check whether it is just a generic resource depot.

Edit: I guess the user should use instanceof ResourceDepot in that case then? Hmm.

This might be a somewhat related or unrelated question: can a worker return resources to an infested Command Center? If so, it would also be considered a ZergResourceDepot, no?

kdlin10 commented 6 years ago

I'm also wary of combinatoric explosion of interfaces, but happily, game mechanics are set so the potential for growth there is limited. I would argue that the interfaces should be limited to things that provide unique functionality, so something like ZergFlyer or ProtossSpellCaster wouldn't qualify, since the only thing flying Zerg units share is that they fly and are Zerg. ZergResourceDepot however, share larva spawning and selecting and passing train commands, so it's a meaningful distinction.

And I don't believe resources can be returned to infested CCs