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

Quick Fix for Closed Issue #204, with the help of @stevealkr #207

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ethanyiwu
Copy link

Add a quick fix patch with the idea of @stevealkr. Using a more simpler format of code to fix the problem.

@ethanyiwu
Copy link
Author

Don't know why there is a single problem on the build package. Only edit one line of code.

@BurnySc2
Copy link
Owner

BurnySc2 commented Dec 17, 2024

Thanks for the pull request!
Can you tell me please when does the error occur?

I'm assuming this is not the only location this error may take place, but everywhere ability_ids are unknown. However with a quick search I couldn't find other places this could happen.

Why should we change it in the list comprehension instead of from_proto function

python-sc2/sc2/unit.py

Lines 87 to 98 in aa247cc

@classmethod
def from_proto(cls, proto: Any, bot_object: BotAI) -> UnitOrder:
target: Optional[Union[int, Point2]] = proto.target_unit_tag
if proto.HasField("target_world_space_pos"):
target = Point2.from_proto(proto.target_world_space_pos)
elif proto.HasField("target_unit_tag"):
target = proto.target_unit_tag
return cls(
ability=bot_object.game_data.abilities[proto.ability_id],
target=target,
progress=proto.progress,
)

For example we could change the return statement to

        return cls(
            ability=bot_object.game_data.abilities[proto.ability_id]
            if proto.ability_id in bot_object.game_data.abilities
            else bot_object.game_data.abilities[0],
            target=target,
            progress=proto.progress,
        )

In one case, the ability would not show up in orders while in my proposed solution we have an unknown ability in the list. Both solutions don't seem great, but both prevent bot crashes of course.

Edit:
The failed CI job shouldn't have been in the first place, so don't worry about it. Packages pushes to pypi should only happen in develop branch so thanks for pointing that out.

Edit2:
The initial issue #204 does claim that the unit type Probe has an unknown ability in the order list.

│ │ │ │ │ │ └ Unit(name='Probe', tag=4350279681)
│ │ │ │ │ └ ability_id: 4132
│ │ │ │ │ target_unit_tag: 4358406145
│ │ │ │ │
│ │ │ │ └ <sc2_rl_agent.starcraftenv_test.env.bot.Protoss_bot.Protoss_Bot object at 0x1062341c0>
│ │ │ └ Unit(name='Probe', tag=4350279681)
│ │ └ ability_id: 4132
│ │ target_unit_tag: 4358406145

All the orders a Probe may have are: move, hold position, attack, gather, return resource and all the build-abilities.
Where does the new ability come from, when there have not been new buildings introduced in the patch?
Have you run your bot on a custom map? In that case, I do not think I want to accept the pull request but rather you should update your ids by running https://github.com/BurnySc2/python-sc2/blob/develop/generate_id_constants_from_stableid.py after you have opened the map once (the stableid.json will be overwritten after you opened the map in SC2 once).

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