-
Notifications
You must be signed in to change notification settings - Fork 228
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
Remove network downloading feature #387
Remove network downloading feature #387
Conversation
5fe05e8
to
9e18d67
Compare
@@ -47,11 +47,10 @@ LizzieFrame.prompt.sgfExists=The SGF file already exists, do you want to replace | |||
LizzieFrame.prompt.showControlsHint=hold x = view controls | |||
LizzieFrame.prompt.switching=switching... | |||
LizzieFrame.display.lastMove=Last move | |||
LizzieFrame.display.pondering=Pondering | |||
LizzieFrame.display.pondering=Pondering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a space is removed here, then a space is added in the LizzieFrame for draw the pondering text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, fixed in the latest commit!
5a8df89
to
4dfb630
Compare
@zsalch Are you done with the review? (Please write a LGTM when you are 😄) |
@@ -1,110 +0,0 @@ | |||
package featurecat.lizzie; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope to keep Util.java for some general functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only method left after removing the network downloading is truncateStringByWidth
, and I feel that this is no really a general util function but instead something that belongs to gui.LizzieFrame
. (It's about fitting text in the screen, it takes a FontMetrics
argument).
What do you think about removing it for now, and adding it again once we have actual general function that wouldn't be better defined in any of the existing files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok.
String ponderingText = resourceBundle.getString("LizzieFrame.display.pondering"); | ||
String switching = resourceBundle.getString("LizzieFrame.prompt.switching"); | ||
String switchingText = Lizzie.leelaz.switching() ? switching : ""; | ||
String weightText = Lizzie.leelaz.currentWeight().toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The currentWeight() is a String. Why need toString here?
LGTM. |
Based on #386, only the last commit is new (I'm happy to update the PR to be only based on next if it makes things simpler).
Downloading the latest network is currently broken, see #268.
In my opinion it makes little sense to have this built in into lizzie, it's much simpler to explain to users that they can download anything from http://zero.sjeng.org/ to replace the
network.gz
that we ship with the release, and they should be good to go!