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

s2gs processing can infinite loop sometimes #59

Closed
dsjoerg opened this issue May 27, 2012 · 6 comments
Closed

s2gs processing can infinite loop sometimes #59

dsjoerg opened this issue May 27, 2012 · 6 comments
Assignees

Comments

@dsjoerg
Copy link
Collaborator

dsjoerg commented May 27, 2012

For example on this US one: 104eef5d50e73f17523225dd262e0b88b4949b108b3d79bba469fff670228770.s2gs

resources.py load_lobby_properties() has a while len(left_lobby) > 0 loop.
Nothing prevents it from looping infinitely, and for the given s2gs file, it does.

I don't understand exactly which assumption made by the author turned out not to be true.

@GraylinKim
Copy link
Owner

@Prillan would know.

It seems that the properties have a series of dependencies which must be loaded before you can determine if the property is "active" or not. Given that understanding, it would seem as though it was assumed that all lobby properties would be eventually resolved, one way or the other.

That being said, I see that there is already a hack in place for this exact problem down below it here.

# Use this to avoid an infinite loop
last_success = 0
max = len(left_players)
while len(left_players) > 0 and not (last_success > max+1):

Perhaps a similar patch should be made for now until we better understand the lobby properties.

@Prillan
Copy link
Collaborator

Prillan commented May 28, 2012

Yes, the hack is there because I was afraid that my implementation of the parsing might cause an infinite loop.

It looks like I forgot it at the place you pointed out...

I'll take a look in a few days when I have time. Thanks for finding it!

@ghost ghost assigned Prillan May 28, 2012
@Prillan
Copy link
Collaborator

Prillan commented Jun 15, 2012

Ok, so I added the same hack to the general properties as well. I think it's the simplest way to do it and doesn't hurt performance too much.
If it does we'll have to fix it but I don't think it's necessary.

Btw, how do I add the commit to this issue?

@GraylinKim
Copy link
Owner

You can tie commits to issues with commit messages. See the github announcement for details.

Relevant Excerpt:

Commits + Issues

Issues has deep integration with commit messages. Any time you reference an issue number from a commit message, we'll bring in the commits to the discussion view for you.

And of course you can close issues with commit messages.

We support a number of synonyms:

  • fixes #xxx
  • fixed #xxx
  • fix #xxx
  • closes #xxx
  • close #xxx
  • closed #xxx

@GraylinKim
Copy link
Owner

Since you've already made the commit, you can also just reference it in the issue text with the commit id as show below:

Patched with 5eca833798913e85032f197185920611897e6b1b

renders as:

Patched with 5eca833

@Prillan
Copy link
Collaborator

Prillan commented Jul 2, 2012

I should probably close this now :)

@Prillan Prillan closed this as completed Jul 2, 2012
StoicLoofah referenced this issue in StoicLoofah/sc2reader May 18, 2018
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

No branches or pull requests

3 participants