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

2017.2 crashes in AI code when player/ship name contains non-ascii characters #2699

Closed
LinuxDonald opened this Issue Jun 1, 2017 · 17 comments

Comments

Projects
None yet
3 participants
@LinuxDonald
Member

LinuxDonald commented Jun 1, 2017

I have become an mail about an crash on the 2017.2 release version on Windows 10:

Traceback (most recent call last):
  File "run_uh.py", line 380, in <module>
    main()
  File "run_uh.py", line 190, in main
    ret = horizons.main.start(options)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\main.py", line 247, in start
    horizons.globals.fife.run()
  File "C:\Unknown-Horizons\unknown-horizons\horizons\engine\engine.py", line 279, in run
    self.loop()
  File "C:\Unknown-Horizons\unknown-horizons\horizons\engine\engine.py", line 291, in loop
    f()
  File "C:\Unknown-Horizons\unknown-horizons\horizons\timer.py", line 113, in check_tick
    f(self.tick_next_id)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\scheduler.py", line 96, in tick
    callback.callback()
  File "C:\Unknown-Horizons\unknown-horizons\horizons\util\python\callback.py", line 47, in __call__
    return self.callback(*self.args, **self.kwargs)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\__init__.py", line 332, in tick_long
    self.strategy_manager.tick()
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\strategy\strategymanager.py", line 334, in tick
    self.handle_strategy()
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\strategy\strategymanager.py", line 319, in handle_strategy
    mission = self.owner.behavior_manager.request_strategy(**environment)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\behavior\__init__.py", line 88, in request_strategy
    return self.request_behavior(type, strategy_name, self.profile.strategies, **environment)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\behavior\__init__.py", line 82, in request_behavior
    return getattr(final_action, action_name)(**environment)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\behavior\behaviorcomponents.py", line 617, in neutral_player
    self.handle_diplomacy(self.parameters_neutral, **environment)
  File "C:\Unknown-Horizons\unknown-horizons\horizons\ai\aiplayer\behavior\behaviorcomponents.py", line 509, in handle_diplomacy
    format(self.owner.name, player.name, balance, relationship_score, action))
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf6' in position 15: ordinal not in range(128)

I created an branch from the 2017.2 release commit. Please fix the issue on this branch to.
Dont forget you need last stable fife and fifechan for it and python2.

@AndyMender @squiddy

@squiddy squiddy added the B-crash label Jun 1, 2017

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 1, 2017

Member

This looks like the same issue as #2130

Member

squiddy commented Jun 1, 2017

This looks like the same issue as #2130

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 1, 2017

Member

Also, just from a quick look at all the self.log.debug in horizons/ai/aiplayer/behavior/behaviorcomponents.py alone I'm pretty sure this will break everywhere, granted you choose to use non-ascii characters in the player's or ship's name.

Member

squiddy commented Jun 1, 2017

Also, just from a quick look at all the self.log.debug in horizons/ai/aiplayer/behavior/behaviorcomponents.py alone I'm pretty sure this will break everywhere, granted you choose to use non-ascii characters in the player's or ship's name.

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 1, 2017

Member

I created an branch from the 2017.2 release commit.

I don't think there is any need for that, we do have tags for that. https://github.com/unknown-horizons/unknown-horizons/tree/2017.2

Member

squiddy commented Jun 1, 2017

I created an branch from the 2017.2 release commit.

I don't think there is any need for that, we do have tags for that. https://github.com/unknown-horizons/unknown-horizons/tree/2017.2

@squiddy squiddy changed the title from 2017.2 crash issue to 2017.2 crashes in AI code when player/ship name contains non-ascii characters Jun 1, 2017

@AndyMender

This comment has been minimized.

Show comment
Hide comment
@AndyMender

AndyMender Jun 2, 2017

Member

How do we approach this then? Do we use a decoder that can handle non-ascii characters or print a warning (or pop-up widget?) that non-ascii characters are not allowed? :)

Member

AndyMender commented Jun 2, 2017

How do we approach this then? Do we use a decoder that can handle non-ascii characters or print a warning (or pop-up widget?) that non-ascii characters are not allowed? :)

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 2, 2017

Member

I think it probably is the same fix as for #2130 which I linked: f96b802

Member

squiddy commented Jun 2, 2017

I think it probably is the same fix as for #2130 which I linked: f96b802

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 2, 2017

Member

Honestly thought, I'd rather concentrate on getting 2017.3 out. With python3's unicode by default, problems like this shouldn't happen.

Member

squiddy commented Jun 2, 2017

Honestly thought, I'd rather concentrate on getting 2017.3 out. With python3's unicode by default, problems like this shouldn't happen.

@LinuxDonald

This comment has been minimized.

Show comment
Hide comment
@LinuxDonald

LinuxDonald Jun 2, 2017

Member

Version 2017.3 is planned for the end of the year. Fife the same.

Member

LinuxDonald commented Jun 2, 2017

Version 2017.3 is planned for the end of the year. Fife the same.

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 2, 2017

Member

Do we really want to wait so long? We've completed a major change, the migration to python 3. The sooner we get feedback, the better, right?

Member

squiddy commented Jun 2, 2017

Do we really want to wait so long? We've completed a major change, the migration to python 3. The sooner we get feedback, the better, right?

@squiddy

This comment has been minimized.

Show comment
Hide comment
@squiddy

squiddy Jun 2, 2017

Member

Sure, we can fix this single bug. But I don't think it's worth the time to check the whole codebase for more issues like that.

Member

squiddy commented Jun 2, 2017

Sure, we can fix this single bug. But I don't think it's worth the time to check the whole codebase for more issues like that.

@LinuxDonald

This comment has been minimized.

Show comment
Hide comment
@LinuxDonald

LinuxDonald Jun 2, 2017

Member

The Problem is that we dont have big changes. Python 3 Port is an big change but its not an big change for the gamer. The Main Problem is still the broken multiplayer for years now :(
And helios2k is not avaliable at the moment. He wanted todo some work on fife. and jakock too (he want port fife from tinyxml to pugixml)

Member

LinuxDonald commented Jun 2, 2017

The Problem is that we dont have big changes. Python 3 Port is an big change but its not an big change for the gamer. The Main Problem is still the broken multiplayer for years now :(
And helios2k is not avaliable at the moment. He wanted todo some work on fife. and jakock too (he want port fife from tinyxml to pugixml)

@LinuxDonald

This comment has been minimized.

Show comment
Hide comment
@LinuxDonald

LinuxDonald Jun 2, 2017

Member

I will talk with the Bug report user. To workaround this Bug. I open an new ticket for new release discuss.

Member

LinuxDonald commented Jun 2, 2017

I will talk with the Bug report user. To workaround this Bug. I open an new ticket for new release discuss.

@AndyMender

This comment has been minimized.

Show comment
Hide comment
@AndyMender

AndyMender Jun 3, 2017

Member

I agree with squiddy. Except for the multiplayer, which doesn't work yet anyway, most of the stuff is in a good enough condition to push for 2017.3. If the unicode issue is gone with Py3, fixing it explicitly for Py2 across the entirety of the codebase might not be a good idea.

Member

AndyMender commented Jun 3, 2017

I agree with squiddy. Except for the multiplayer, which doesn't work yet anyway, most of the stuff is in a good enough condition to push for 2017.3. If the unicode issue is gone with Py3, fixing it explicitly for Py2 across the entirety of the codebase might not be a good idea.

@LinuxDonald

This comment has been minimized.

Show comment
Hide comment
@LinuxDonald

LinuxDonald Jun 3, 2017

Member

I cant do an release again with nothing new. We have only codebase changes. I listen only from other that we dont have anything new in our releases only small changes for years now.
I get feedback from players. And they all say they love our game. And they will be happy to see an new version. But are they happy to see an release with nothing new related to an gamer?
When you guys want i can make an new release after Sommer. I must talk first with helios2k about new fife/fifechan release. When MP is still broken than we need to disable it for release.

Member

LinuxDonald commented Jun 3, 2017

I cant do an release again with nothing new. We have only codebase changes. I listen only from other that we dont have anything new in our releases only small changes for years now.
I get feedback from players. And they all say they love our game. And they will be happy to see an new version. But are they happy to see an release with nothing new related to an gamer?
When you guys want i can make an new release after Sommer. I must talk first with helios2k about new fife/fifechan release. When MP is still broken than we need to disable it for release.

@AndyMender

This comment has been minimized.

Show comment
Hide comment
@AndyMender

AndyMender Jun 3, 2017

Member

Well, we're coders so what changes mostly is the codebase. I started working in Blender again so maybe I'll be able to push in some buildings, but apart from me there is only Jana in the "content department". Perhaps we could advertise the project a bit more?

Still, that's for a separate issue, I believe.

Member

AndyMender commented Jun 3, 2017

Well, we're coders so what changes mostly is the codebase. I started working in Blender again so maybe I'll be able to push in some buildings, but apart from me there is only Jana in the "content department". Perhaps we could advertise the project a bit more?

Still, that's for a separate issue, I believe.

@LinuxDonald

This comment has been minimized.

Show comment
Hide comment
@LinuxDonald

LinuxDonald Jun 3, 2017

Member

We dont need new content we have enough. We have alot work that coders only can do. For example ship fight. We have already cannon balls grafics. We have alot issuse with features that needs coders to get done.

Member

LinuxDonald commented Jun 3, 2017

We dont need new content we have enough. We have alot work that coders only can do. For example ship fight. We have already cannon balls grafics. We have alot issuse with features that needs coders to get done.

@squiddy squiddy added this to the 2017.2.1 milestone Jun 13, 2017

@AndyMender

This comment has been minimized.

Show comment
Hide comment
@AndyMender

AndyMender Jul 19, 2017

Member

@LinuxDonald can be closed as "will be fixed in the next release" or should we leave it open still?

Member

AndyMender commented Jul 19, 2017

@LinuxDonald can be closed as "will be fixed in the next release" or should we leave it open still?

@LinuxDonald

This comment has been minimized.

Show comment
Hide comment
@LinuxDonald

LinuxDonald Jul 19, 2017

Member

Maybe squidds will do an backport.

Member

LinuxDonald commented Jul 19, 2017

Maybe squidds will do an backport.

@LinuxDonald LinuxDonald modified the milestones: 2017.2.1, 2018.1 Apr 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment