Open StoicLoofah opened 10 years ago
It may be related to #174.
So the issue is as described: observers don't have units. For this particular replay, the offending observer is Observer 10 - makoz
and unit is BeaconArmy [2A40001]
There are 2 fixes I see here. One, only append the unit if the owner has units or is a player. It's a little messy and would need to get applied in a lot of places, but it could be a check like
isinstance(..., Player)
hasattr(..., 'units').
OR
Two, have Observers get units, either
self.users = list()
class Observer(Entity, User, Player):
Slapping the users attribute in directly worked in my testing. Does anyone have strong preferences in the right fix here?
Are Observers actually allowed to own units? I believe the answer is no, and if so, then I would recommend also adding a more explicit error message saying: "WARNING: this replay seems to show an Observer (makoz) owning a unit (BeaconArmy), which is impossible, so there is something wrong with replay parsing."
Secondly, if indeed Observers aren't allowed to own units, then there is some deeper bug, either on the sc2reader side or some limitation in what we can get from the info Blizzard provides.
On Sun, Aug 10, 2014 at 2:27 PM, Kevin Leung notifications@github.com wrote:
So the issue is as described: observers don't have units. For this particular replay, the offending observer is Observer 10 - makoz and unit is BeaconArmy [2A40001]
There are a few possible fixes here
- only append the unit if the owner has units or is a player. It's a little messy and would need to get applied in a lot of places, but it could be a check like * isinstance(..., Player) * hasattr(..., 'units').
- have Observers get units, either * add it to the constructor explicitlyself.users = list()` * have Observer inherit from Player as well
Slapping the users attribute in directly worked in my testing. Does anyone have strong preferences in the right fix here?
— Reply to this email directly or view it on GitHub https://github.com/GraylinKim/sc2reader/issues/176#issuecomment-51722809 .
I have no idea what is and isn't valid here. @GraylinKim , do you know what should be allowed for observers with units?
@dsjoerg is right @StoicLoofah. Observers are not allowed to own units. This rule is enforced by the Starcraft II game engine. So the client that sc2reader is identifying as an observer is actually a player as far as the map used and game engine are concerned.
sc2reader is either improperly identifying a player as an observer or it needs to have better handling for situations where observers are registered as players. In the case of the former it is a logic bug we should try to fix. In the case of the later it is an event processing bug that should be fixed. Since real observers can't have units, I'd be in favor of dropping all "observer" unit events and not processing them.
I doubt that this is exactly related to #174 @EHadoux but it is likely the same class of problem. I don't have time to look into these issues right now but I can try to provide guidance on a fix.
The entities on this particular replay are
{1: Player 1 - Major (Terran), 2: Player 2 - Minigun (Protoss), 3: Observer 10 - makoz, 4: Observer 9 - Emil, 5: Observer 7 - TaKeSeN, 6: Observer 5 - ToD, 7: Observer 4 - TKL, 8: Observer 3 - Hui, 9: Observer 2 - CHL, 10: Observer 1 - Destiny, 11: Observer 0 - Hunky}
And I spit out the upkeep_pids from the replay (as initializing trackerevents in https://github.com/GraylinKim/sc2reader/blob/master/sc2reader/events/tracker.py#L269 ) , and there were a lot of 0s, 3s, and 4s. As a sample,
BeaconArmy 4
BeaconDefend 4
BeaconAttack 4
BeaconHarass 4
BeaconIdle 4
BeaconAuto 4
BeaconDetect 4
BeaconScout 4
BeaconClaim 4
BeaconExpand 4
BeaconRally 4
BeaconCustom1 4
BeaconCustom2 4
BeaconCustom3 4
BeaconCustom4 4
CommandCenter 3
SCV 3
SCV 3
SCV 3
SCV 3
SCV 3
SCV 3
Nexus 4
Probe 4
Probe 4
Probe 4
Probe 4
Probe 4
Probe 4
Probe 4
which is from something like print('{} {}'.format(self.unit_type_name, data[3]))
. I would assume that makoz and emil aren't the upkeepers on these very regular units, so something is up.
It would seem to me that the problem is either
I have no idea what the format of the tracker data is or where to find such a thing to even start here. Thoughts on next steps?
I'm guessing this is related to https://github.com/Blizzard/s2protocol/issues/8.
If my guess is right, the issue is related to how we figure out the mapping from PIDs to players. I believe sc2reader uses a technique that works for ladder games but doesn't always work on custom maps and/or games with observers.
Take a look at the discussion in https://github.com/Blizzard/s2protocol/issues/8, in particular @koalaling's response:
"The short answer is there's no great way to do this right now; it requires information from the map file to compute player ids. However it's a good idea to add new tracker events in a patch to record the mapping between player id, user id and lobby slot id."
So, possible approaches include: 1) parse the map to improve the player -> PID mapping (however see @GraylinKim's last comment in that issue, indicating that even this may not be a silver bullter) 2) add some more heuristics to the player -> PID inference to correctly handle cases like this, e.g. PIDs that own units have to be Players, if there are only two players of different races, we can figure out which is which based on which units they own.
@koalaling writes:
There’s two SPlayerSetupEvents with m_type = 1 (meaning ‘user’, i.e. a human player) in the tracker events:
{'_bits': 192, '_event': 'NNet.Replay.Tracker.SPlayerSetupEvent', '_eventid': 9, '_gameloop': 0, 'm_playerId': 3, 'm_slotId': 2, 'm_type': 1, 'm_userId': 6} {'_bits': 192, '_event': 'NNet.Replay.Tracker.SPlayerSetupEvent', '_eventid': 9, '_gameloop': 0, 'm_playerId': 4, 'm_slotId': 3, 'm_type': 1, 'm_userId': 8}
The SPlayerSetupEvents can be used to map userIds to playerIds: userId=6 maps to playerId=3, and slotId=2 userId=8 maps to playerId=4, and slotId=3
m_userInitialData has player names, and is indexed by the userId. m_userInitialData[userId=6].m_name is ‘Major’ m_userInitialData[userId=8].m_name is ‘Minigun’
There’s more info about players in m_lobbyState.m_slots[], which is indexed by slotId. For example: {'m_aiBuild': 0, 'm_colorPref': {'m_color': 2}, 'm_control': 2, 'm_difficulty': 3, 'm_handicap': 100, 'm_licenses': [162, 259], 'm_observe': 0, 'm_racePref': {'m_race': 0}, 'm_rewards': [2686638163L, 1333792768, 2009110693, 656055948, 1542695810, 18730036, 2820704051L, 616156284, 2584760982L, 3834465703L, 2276535543L, 4241103608L, 1127476957, 928246938, 2419802213L, 3404898509L, 2063317397], 'm_teamId': 0, 'm_toonHandle': '1-S2-1-3819890', 'm_userId': 6, 'm_workingSetSlotId': 8},
There’s also the m_playerList, but rather than indexing into it, you can correlate the m_workingSetSlotId in those objects with the same-named field in objects in the m_lobbyState.m_slots[] entries. For example: {'m_color': {'m_a': 255, 'm_b': 255, 'm_g': 66, 'm_r': 0}, 'm_control': 2, 'm_handicap': 100, 'm_name': 'Major', 'm_observe': 0, 'm_race': 'Terran', 'm_result': 1, 'm_teamId': 0, 'm_toon': {'m_id': 3819890, 'm_programId': '\x00\x00S2', 'm_realm': 1, 'm_region': 1}, 'm_workingSetSlotId': 8},
Sorry for not following up on this sooner, but I have done a bit more sleuthing to get through this.
Okay, so what I have got out of this replay is
from the SPlayerSetupEvent
pid 0 uid None sid None
pid 3 uid 6 sid 2
pid 4 uid 8 sid 3
pid 15 uid None sid None
from replay.entities
Player 1 - Major (Terran) pid 1 uid 6 sid 2
Player 2 - Minigun (Protoss) pid 2 uid 8 sid 3
Observer 10 - makoz pid 3 uid 10 sid 7
Observer 9 - Emil pid 4 uid 9 sid 8
Observer 7 - TaKeSeN pid 5 uid 7 sid 9
Observer 5 - ToD pid 6 uid 5 sid 10
Observer 4 - TKL pid 7 uid 4 sid 11
Observer 3 - Hui pid 8 uid 3 sid 12
Observer 2 - CHL pid 9 uid 2 sid 13
Observer 1 - Destiny pid 10 uid 1 sid 14
Observer 0 - Hunky pid 11 uid 0 sid 15
from lobby_state slots
0 None {'control': 0, 'rewards': [], 'user_id': None, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 1, 'difficulty': 3, 'team_id': 0, 'observe': 0, 'race_pref': 0, 'licenses': [], 'working_set_slot_id': 5, 'toon_handle': u''}
1 None {'control': 0, 'rewards': [], 'user_id': None, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 1, 'difficulty': 3, 'team_id': 1, 'observe': 0, 'race_pref': 0, 'licenses': [], 'working_set_slot_id': 6, 'toon_handle': u''}
2 6 {'control': 2, 'rewards': [2686638163L, 1333792768L, 2009110693L, 656055948L, 1542695810L, 18730036L, 2820704051L, 616156284L, 2584760982L, 3834465703L, 2276535543L, 4241103608L, 1127476957L, 928246938L, 2419802213L, 3404898509L, 2063317397L], 'user_id': 6, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 2, 'difficulty': 3, 'team_id': 0, 'observe': 0, 'race_pref': 0, 'licenses': [162L, 259L], 'working_set_slot_id': 8, 'toon_handle': u'1-S2-1-3819890'}
3 8 {'control': 2, 'rewards': [2359737029L, 18730036L, 1467565626L, 1916636256L, 2222924803L, 3399536420L, 4241103608L, 2976815359L, 1127476957L, 2419802213L], 'user_id': 8, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 1, 'difficulty': 3, 'team_id': 1, 'observe': 0, 'race_pref': 2, 'licenses': [162L, 259L], 'working_set_slot_id': 10, 'toon_handle': u'1-S2-1-288081'}
4 None {'control': 1, 'rewards': [], 'user_id': None, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [], 'working_set_slot_id': 15, 'toon_handle': u''}
5 None {'control': 1, 'rewards': [], 'user_id': None, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [], 'working_set_slot_id': 14, 'toon_handle': u''}
6 None {'control': 0, 'rewards': [], 'user_id': None, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [], 'working_set_slot_id': 13, 'toon_handle': u''}
7 10 {'control': 2, 'rewards': [], 'user_id': 10, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [162L, 259L], 'working_set_slot_id': 12, 'toon_handle': u'1-S2-1-3787027'}
8 9 {'control': 2, 'rewards': [], 'user_id': 9, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [162L, 259L], 'working_set_slot_id': 11, 'toon_handle': u'1-S2-1-2443002'}
9 7 {'control': 2, 'rewards': [], 'user_id': 7, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [162L, 259L], 'working_set_slot_id': 9, 'toon_handle': u'1-S2-1-3281854'}
10 5 {'control': 2, 'rewards': [], 'user_id': 5, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [162L, 259L], 'working_set_slot_id': 7, 'toon_handle': u'1-S2-1-2580448'}
11 4 {'control': 2, 'rewards': [], 'user_id': 4, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 1, 'observe': 1, 'race_pref': 2, 'licenses': [162L, 259L], 'working_set_slot_id': 4, 'toon_handle': u'1-S2-1-4643678'}
12 3 {'control': 2, 'rewards': [], 'user_id': 3, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 1, 'observe': 1, 'race_pref': 2, 'licenses': [162L, 259L], 'working_set_slot_id': 3, 'toon_handle': u'1-S2-1-3667996'}
13 2 {'control': 2, 'rewards': [], 'user_id': 2, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 1, 'observe': 1, 'race_pref': None, 'licenses': [162L, 259L], 'working_set_slot_id': 2, 'toon_handle': u'1-S2-1-1497486'}
14 1 {'control': 2, 'rewards': [], 'user_id': 1, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 1, 'licenses': [162L, 259L], 'working_set_slot_id': 1, 'toon_handle': u'1-S2-1-310150'}
15 0 {'control': 2, 'rewards': [], 'user_id': 0, 'logo_index': None, 'handicap': 100, 'ai_build': 0, 'colorPref': 0, 'difficulty': 3, 'team_id': 0, 'observe': 1, 'race_pref': 0, 'licenses': [162L, 259L], 'working_set_slot_id': 0, 'toon_handle': u'1-S2-1-375989'}
Presumably, the pids usually match up between these 2 sources, but they didn't in this case for some of the reasons @koalaling discussed above.
It appears that the pids (or player_id in load_players) as defined in sc2reader/resources.py are simply enumerated counting up from 1. In this case, however, it turns out that the first 2 players in the replay are actually pids 3 and 4 according to the tracker events, and looking at the slots data, there is nothing to indicate that they are players 3 and 4.
The suggestion above to use tracker events is a little unfortunate because it looks like sc2reader loads the slot data before it even looks at the tracker data, and given that tracker data isn't available in all cases, I'm wondering whether it's really kosher to reassign pids after the fact. I could try it, but it really seems like this is the edge case here.
Am I missing something else in the structure of the slotData that allows us to do this more consistently? The only guess I have is that we have to increment the player_id for the 2 slots at the beginning with control == 0. I'm not exactly sure how to interpret "control", however, or if that will break more things. Or maybe we're supposed to ignore the control altogether and increment it in all cases? If so, then pid is simply sid + 1
I got the following error:
http://spawningtool.com/11855/download/
To be honest, I haven't taken a look at this issue at all and am happy to investigate, but I just pulled up the error right now and thought I would put it out there in case anyone knows what's up off the top of his or her head