OpenBW / BWAPI4J

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

Add features to circumvent BWAPI's unit data invalidation? #85

Open adakitesystems opened 6 years ago

adakitesystems commented 6 years ago

Example of a removed feature: https://github.com/OpenBW/BWAPI4J/pull/84/commits/259555eb818b23f24fae505fa9af142ae768fca1

If an enemy unit moves out of vision, BWAPI hides most the unit's data. Such examples are marking its position as Position::None or Position::Unknown or possibly crashing when trying to access certain data. Should BWAPI4J provide "lastKnownXXXX" data in the Unit object or leave it up to the user for such an implementation?

JasperGeurtz commented 6 years ago

I think it's a good addition to have them in extra separate methods in Unit, almost every bot author will find it useful to track where a unit from the opponent was last seen.

adakitesystems commented 6 years ago

That's a fair argument. Having "lastKnown" data is useful without a doubt. How to provide such data in a useful but extendible way is interesting. I would say the Unit is divided into at least three or four parts:

  1. Data and their getters (e.g. hitpoints, shields, unit type, etc.)
  2. More complex state-observant getters such as getGroundWeaponCooldown that considers the game state (e.g. Zergling attack speed upgrade).
  3. Actions (issue commands, e.g. gather, etc.)
  4. Calculators (e.g. getDistance or getClosestUnit)

In this specific case, the lastKnown dataset is just a copy of a previous valid dataset before the unit moved out of vision. I think I might attempt to internally divide the Unit into the above mentioned parts (or at least divide the data into a separate structure for now) such that copies can be made and modified which could be useful for running simulations or moving a game state into the future for prediction.

dgant commented 6 years ago

I think it best to do the simplest thing (forward BWAPI's data) and push improvements to the reported data upstream to BWAPI itself. The handling of previously seen units carries some inherent subjectivity (is the unit's presumed location where it was last seen? moving along the path it was following? what if you see the location later and the unit's not there? what if you can infer that the unit isn't there because it was a Spider Mine and you just walked over it? etc.)

Most authors beyond some level of sophistical will want/need to implement their own proxies on top of Unit anyhow. The primary need for consumers of alternative language wrappers is the most complete and accurate reporting of game data possible. BWAPI4J is serving excellently if it can do that. Further utility is better left for smaller libraries that can move faster and aren't coupled to the wrapper like https://github.com/andertavares/StarCraftUnitInfo

Bytekeeper commented 6 years ago

All the lastXYZ was not useful anyways. Currently units just keep their last state if they're no longer visible (with the exception of isVisible and exists). I kind of like it that way.

Therefore I don't like the removal of lastSpotted because it makes any kind of prediction harder. I also don't think it's really subjective to keep the "last known state" in a Unit. Especially since it's no longer being reported by BWAPI4J when accessing units.