Skip to content

Conversation

@endor
Copy link

@endor endor commented Sep 13, 2012

No description provided.

@justinweiss
Copy link
Collaborator

Hey, I just got a chance to build this version and put it on my phone. I'll use it and let you know what I think!

@justinweiss
Copy link
Collaborator

As for the test stuff, DGS sometimes calculates move numbers differently than fuego does -- you can see the results of some of that in the FuegoBoard class. It's kind of a mess. Starting with their new API, DGS returns their idea of the current move number in a special property of the SGF. I haven't figured out how to get Fuego to read it yet, though.

My tests for this project are pretty embarrassing, but in my defense, testing in older versions of Xcode was really, really painful. I've heard it's much better now, but I haven't had a chance to look into how it's improved yet. I wouldn't use my existing tests as a good example of how they should be written. I'd go straight to the docs, instead.

@justinweiss
Copy link
Collaborator

OK, I had a chance to take a look at this. In general, the code looks great, especially for someone who hasn't done much iOS development. I had a few questions about the implementation, though, and one bug I noticed.

  1. The list -> detail pattern makes sense in most cases, but I'm not sure it fits here. The most common case I'd use this for is looking at the most recent 5-10 moves, and I'd probably want to scan those rather than tapping 'back' and the move I want to look at. Have you thought about maybe bringing them directly to the view of the most recent move, and having 'next' and 'prev' controls on the toolbar on that screen? I think that would make my most common case easier, and you could still have the history list accessible from that screen if someone wanted to jump right to another move.
  2. Going along with that, if none of the controls on that screen are going to be enabled, I'd just remove them (and the code behind them). It's less confusing that way, and it'll simplify the code and eliminate a lot of the duplicated logic in your movecontroller.
  3. Whenever you hit the move list, the UI freezes for a second. It looks like you're re-parsing the board every time you see that view, which is unnecessary - I'd just do it once, when the board loads. If you wanted to be really awesome, you could even do the board parsing in a background thread, so you don't lock the UI - take a look in the iOS documentation for dispatch_async or NSOperation.
  4. When you hit the move view list and go back to the game, zoom appears to be broken - it won't zoom anymore. It'll still let you play your move, though.

Other than that, I like it. Thanks!

@endor
Copy link
Author

endor commented Sep 20, 2012

  1. That sounds very reasonable. I will try to do that.

  2. Well.. the point here is.. I tried to write my own view by copying the one that existed and then removing the elements I don't want but for some reason that was very difficult. Setting up a view with the board I mean. So it seemed easier to copy some code and not copy the whole view but just reuse the existing one. I guess if I want to have prev/next buttons I will need to use my own view anyway. Let's see if that will work.

  3. Uhm, yeah.. also reasonable, but I am not sure if I will be able to handle that right away.

  4. Ah, hadn't noticed that. Thanks!

@justinweiss
Copy link
Collaborator

Ah, I totally didn't think about how the board interacts with the controller. I probably could have designed it better. Could you go into more detail about what wasn't working? At one point, I was considering enabling a 'read-only' version of the board view that wouldn't interact as much with the controller, and it would be nice to have a list of the stuff that wasn't working, so I'd have a better idea of what needs to go.

@endor
Copy link
Author

endor commented Sep 27, 2012

Maybe you can have a look at the latest commit in my branch. I just can't seem to get zooming to work if I use my own view. Maybe it's because I copied it fron the other nib file?

@justinweiss
Copy link
Collaborator

This is how zooming works:

  • A touch is detected in BoardView
  • BoardView calles its delegate's (void)handleGoBoardTouch:(UITouch *)touch inView:(GoBoardView *)view method
  • That method calls zoomToScale or playStoneAtPoint or whatever

Without looking at it, my guess would be that you haven't connected the delegate relationship in your xib file (to set up PastMoveViewController as the delegate of your new BoardViewController), or that you don't have any code inside PastMoveViewController's handleGoBoardTouch:inView: method. I'll be able to check your branch out and take a closer look soon, hopefully :-)

Thanks,
Justin

On Sep 27, 2012, at 1:27 AM, Frank Prößdorf [email protected] wrote:

Maybe you can have a look at the latest commit in my branch. I just can't seem to get zooming to work if I use my own view. Maybe it's because I copied it fron the other nib file?


Reply to this email directly or view it on GitHub.

@endor
Copy link
Author

endor commented Oct 2, 2012

Hey Justin,

yeah, I know that bit about zooming and it's not the problem. Actually I am not sure I want to have zooming via touch in this view at all. The real problem is that I can't get the 19x19 board displayed in the correct size.. it's shown zoomed in.. or rather.. not zoomed out. And that is the problem I don't know how to fix. I have some log statements in the code as well to compare it to the GameView but I can't see any differences. Thanks for looking into this.

Frank

On Oct 1, 2012, at 6:25 PM, Justin Weiss [email protected] wrote:

This is how zooming works:

  • A touch is detected in BoardView
  • BoardView calles its delegate's (void)handleGoBoardTouch:(UITouch *)touch inView:(GoBoardView *)view method
  • That method calls zoomToScale or playStoneAtPoint or whatever

Without looking at it, my guess would be that you haven't connected the delegate relationship in your xib file (to set up PastMoveViewController as the delegate of your new BoardViewController), or that you don't have any code inside PastMoveViewController's handleGoBoardTouch:inView: method. I'll be able to check your branch out and take a closer look soon, hopefully :-)

Thanks,
Justin

On Sep 27, 2012, at 1:27 AM, Frank Prößdorf [email protected] wrote:

Maybe you can have a look at the latest commit in my branch. I just can't seem to get zooming to work if I use my own view. Maybe it's because I copied it fron the other nib file?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

@justinweiss
Copy link
Collaborator

Ah, yeah, I actually just looked at this part of the code. The board view inside the scroll view is setup at 100% (640x640) in viewDidLoad. Later, in viewWillAppear, the controller manually zooms out to 50%. Somehow, those two methods drifted away from each other in the controller .m file, so it took me a little while to figure out where that connection was. Does that sound like it could be your problem?

Zooming is kind of a mess inside that controller, anyway. I liked the interaction of tap to zoom instead of pinch to zoom, so the game could be played one-handed. All the locking / unlocking code is just there to disable pinch-to-zoom, and I haven't found a better way of solving that problem yet.

I totally agree with you about disabling zooming in the other view, though. The only reason I have it in the game view controller is because it's nearly impossible to place a stone accurately without it. If you're not placing stones, it's probably not necessary.

On Oct 2, 2012, at 12:32 AM, Frank Prößdorf [email protected] wrote:

Hey Justin,

yeah, I know that bit about zooming and it's not the problem. Actually I am not sure I want to have zooming via touch in this view at all. The real problem is that I can't get the 19x19 board displayed in the correct size.. it's shown zoomed in.. or rather.. not zoomed out. And that is the problem I don't know how to fix. I have some log statements in the code as well to compare it to the GameView but I can't see any differences. Thanks for looking into this.

Frank

On Oct 1, 2012, at 6:25 PM, Justin Weiss [email protected] wrote:

This is how zooming works:

  • A touch is detected in BoardView
  • BoardView calles its delegate's (void)handleGoBoardTouch:(UITouch *)touch inView:(GoBoardView *)view method
  • That method calls zoomToScale or playStoneAtPoint or whatever

Without looking at it, my guess would be that you haven't connected the delegate relationship in your xib file (to set up PastMoveViewController as the delegate of your new BoardViewController), or that you don't have any code inside PastMoveViewController's handleGoBoardTouch:inView: method. I'll be able to check your branch out and take a closer look soon, hopefully :-)

Thanks,
Justin

On Sep 27, 2012, at 1:27 AM, Frank Prößdorf [email protected] wrote:

Maybe you can have a look at the latest commit in my branch. I just can't seem to get zooming to work if I use my own view. Maybe it's because I copied it fron the other nib file?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

@endor
Copy link
Author

endor commented Oct 2, 2012

Yeah, I quite like the tap to zoom and then place stone as well. It's not how most other programs work, but it's very efficient and once you're used to it you don't want to miss it. But yes, the code is really quite complicated and the names don't really explain what the functions do - in my humble opinion. But yes, that sounds like my problem. I know that the 640x640 is done in my controller as well because I have a log statement for that. And I am pretty sure I do a zoom to 0.5 too. But I don't end up zoomed to the 0.5. So.. I do get to the zoom to 0.5 method but for some reason it doesn't do the same thing as the method in the other controller. I also put log statements in there, but they seem to display the same thing as in the other controller. That's why I don't see the difference :/

On Oct 2, 2012, at 9:46 AM, Justin Weiss [email protected] wrote:

Ah, yeah, I actually just looked at this part of the code. The board view inside the scroll view is setup at 100% (640x640) in viewDidLoad. Later, in viewWillAppear, the controller manually zooms out to 50%. Somehow, those two methods drifted away from each other in the controller .m file, so it took me a little while to figure out where that connection was. Does that sound like it could be your problem?

Zooming is kind of a mess inside that controller, anyway. I liked the interaction of tap to zoom instead of pinch to zoom, so the game could be played one-handed. All the locking / unlocking code is just there to disable pinch-to-zoom, and I haven't found a better way of solving that problem yet.

I totally agree with you about disabling zooming in the other view, though. The only reason I have it in the game view controller is because it's nearly impossible to place a stone accurately without it. If you're not placing stones, it's probably not necessary.

On Oct 2, 2012, at 12:32 AM, Frank Prößdorf [email protected] wrote:

Hey Justin,

yeah, I know that bit about zooming and it's not the problem. Actually I am not sure I want to have zooming via touch in this view at all. The real problem is that I can't get the 19x19 board displayed in the correct size.. it's shown zoomed in.. or rather.. not zoomed out. And that is the problem I don't know how to fix. I have some log statements in the code as well to compare it to the GameView but I can't see any differences. Thanks for looking into this.

Frank

On Oct 1, 2012, at 6:25 PM, Justin Weiss [email protected] wrote:

This is how zooming works:

  • A touch is detected in BoardView
  • BoardView calles its delegate's (void)handleGoBoardTouch:(UITouch *)touch inView:(GoBoardView *)view method
  • That method calls zoomToScale or playStoneAtPoint or whatever

Without looking at it, my guess would be that you haven't connected the delegate relationship in your xib file (to set up PastMoveViewController as the delegate of your new BoardViewController), or that you don't have any code inside PastMoveViewController's handleGoBoardTouch:inView: method. I'll be able to check your branch out and take a closer look soon, hopefully :-)

Thanks,
Justin

On Sep 27, 2012, at 1:27 AM, Frank Prößdorf [email protected] wrote:

Maybe you can have a look at the latest commit in my branch. I just can't seem to get zooming to work if I use my own view. Maybe it's because I copied it fron the other nib file?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

@justinweiss
Copy link
Collaborator

OK, I think I see what the problem is. There are a few issues:

  • The scroll view in the xib file needs to have its delegate property set to 'File's Owner'. Otherwise the scroll view won't know which controller to call its methods on.

  • You have to implement the UIScrollViewDelegate method viewForZoomingInScrollView: on the past move controller.

  • The two lines:

    self.scrollView.maximumZoomScale = 1.0;
    self.scrollView.minimumZoomScale = 1.0;

will prevent the scrollView from zooming, so

[self.scrollView zoomToRect:zoomRect animated:animated];

won't do anything.

I can totally see how this would have been hard to debug, since fixing any one (or two!) of these problems would seem to have no effect -- all three had to be fixed together.

Justin

On Oct 2, 2012, at 12:51 AM, Frank Prößdorf [email protected] wrote:

Yeah, I quite like the tap to zoom and then place stone as well. It's not how most other programs work, but it's very efficient and once you're used to it you don't want to miss it. But yes, the code is really quite complicated and the names don't really explain what the functions do - in my humble opinion. But yes, that sounds like my problem. I know that the 640x640 is done in my controller as well because I have a log statement for that. And I am pretty sure I do a zoom to 0.5 too. But I don't end up zoomed to the 0.5. So.. I do get to the zoom to 0.5 method but for some reason it doesn't do the same thing as the method in the other controller. I also put log statements in there, but they seem to display the same thing as in the other controller. That's why I don't see the difference :/

On Oct 2, 2012, at 9:46 AM, Justin Weiss [email protected] wrote:

Ah, yeah, I actually just looked at this part of the code. The board view inside the scroll view is setup at 100% (640x640) in viewDidLoad. Later, in viewWillAppear, the controller manually zooms out to 50%. Somehow, those two methods drifted away from each other in the controller .m file, so it took me a little while to figure out where that connection was. Does that sound like it could be your problem?

Zooming is kind of a mess inside that controller, anyway. I liked the interaction of tap to zoom instead of pinch to zoom, so the game could be played one-handed. All the locking / unlocking code is just there to disable pinch-to-zoom, and I haven't found a better way of solving that problem yet.

I totally agree with you about disabling zooming in the other view, though. The only reason I have it in the game view controller is because it's nearly impossible to place a stone accurately without it. If you're not placing stones, it's probably not necessary.

On Oct 2, 2012, at 12:32 AM, Frank Prößdorf [email protected] wrote:

Hey Justin,

yeah, I know that bit about zooming and it's not the problem. Actually I am not sure I want to have zooming via touch in this view at all. The real problem is that I can't get the 19x19 board displayed in the correct size.. it's shown zoomed in.. or rather.. not zoomed out. And that is the problem I don't know how to fix. I have some log statements in the code as well to compare it to the GameView but I can't see any differences. Thanks for looking into this.

Frank

On Oct 1, 2012, at 6:25 PM, Justin Weiss [email protected] wrote:

This is how zooming works:

  • A touch is detected in BoardView
  • BoardView calles its delegate's (void)handleGoBoardTouch:(UITouch *)touch inView:(GoBoardView *)view method
  • That method calls zoomToScale or playStoneAtPoint or whatever

Without looking at it, my guess would be that you haven't connected the delegate relationship in your xib file (to set up PastMoveViewController as the delegate of your new BoardViewController), or that you don't have any code inside PastMoveViewController's handleGoBoardTouch:inView: method. I'll be able to check your branch out and take a closer look soon, hopefully :-)

Thanks,
Justin

On Sep 27, 2012, at 1:27 AM, Frank Prößdorf [email protected] wrote:

Maybe you can have a look at the latest commit in my branch. I just can't seem to get zooming to work if I use my own view. Maybe it's because I copied it fron the other nib file?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

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