Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow operating on old Unit objects #156

Open
BurnySc2 opened this issue Nov 14, 2022 · 1 comment
Open

Disallow operating on old Unit objects #156

BurnySc2 opened this issue Nov 14, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@BurnySc2
Copy link
Owner

Problem:
Accessing health, position and other attributes that can change in between frames may show different values every X frames.
This means checking unit.is_idle may keep returning the same boolean based on unit.orders.
unit.distance_to(other) will also not return the correct value because the unit.position is outdated, or if the scipy matrices are accessed then the unit objects may have shuffled matrix index values.

Suggestions:
Disallow using distance_to and similar functions when using outdated unit objects. This only fixed the distance calculation.
The same issue persists for unit.orders and unit.health, and authors may confused themselves.

@BurnySc2 BurnySc2 added the bug Something isn't working label Nov 14, 2022
@Zane-Larking
Copy link

I've got a couple of suggestions for reducing confusion.
First create a getter for the _proto in the Unit class that checks the Unit's _bot_object.state.game_loop and game_loop attributes are equal (I see Unit.is_memory exists) and log a warning (or through an exception) if they aren't. Though, I don't know how much overhead this would add.

Note my understanding of the code base is pretty surface level. Sorry if anything I say from here is misleading 😅

With the Unit._proto.orders coming from the protobuf raw message you'd expect them to not change until the next frame. Thus if a Unit receives actions, Unit.is_idle will not update until the following frame. I'd suggest updating the Unit._proto.orders in BotAIInternal.do(). I do see it's a cached property so I'd suggest the following:

Either make 'orders' a (non-cached) property or

  • rename orders to _orders
  • add a unit_orders: Tuple[UnitOrder] to the unit prototype
  • add a proxy called orders that returns the above concatenated:
@property
def orders(self) -> Tuple[UnitOrder, ...]:
        return self._orders + self._proto._unit_orders

Further, though I haven't had the time to myself, I was thinking of adding a subscriber based "Units/Unit refresher class" that would run in the BotAIInternal._prepare_units and update the Unit prototypes of those subscribed (based on tags). You could, potentially, also have it also remove GameState.dead_units (Haven't considered this enough) so the user wouldn't need to manage dead units and structures.

For added performance for the above I'd suggest storing the underlying list of Units in ascending/descending tag order to make Units.find_by_tag() O(log n). This can be done by sorting the BotAIInternal.state.observation_raw.units O(n log n) prior to the bot Units' assignment and modifying the underlying List methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants