SabakiHQ / Sabaki

An elegant Go board and SGF editor for a more civilized age.
https://sabaki.yichuanshen.de/
MIT License
2.39k stars 376 forks source link

Allows users to edit influence boxes when in Estimate/Scoring mode. #941

Open RobertChrist opened 1 year ago

RobertChrist commented 1 year ago

When in scoring or estimation mode, we now track any vertexes that the user clicks via the state.estimateOverrides property. In our app render method, we then override the area influence map with these clicks. Clicks cycle through black/white/ empty. A +1, -1 is used in the equations to reflect the fact that black/white/empty values in the influence map are tracked via values of [-1,0,1].

apetresc commented 7 months ago

Similar apology here about the length of time it's taken me to get to it :)

This is cool, and seems to work well. But I think we need to also update the subroutine for marking stones alive/dead, since the changed values in the areaMap are clashing. See the video below for a demonstration of the issue:

https://github.com/SabakiHQ/Sabaki/assets/14872/bfcf2a96-1d90-4696-9678-d7cc4f9c0db3

I'm not completely sure what should happen in that case, but definitely not that. Probably we should treat the user's override as stable regardless of the changes caused by toggling life/death? Since presumably that's why they're overriding it in the first place.

Do you wanna take a stab at this or should I?

RobertChrist commented 7 months ago

Huh.

Apologies for not catching that, and nice find!

Yeah, it looks like its iterating through the options when the stone is marked as alive/dead, which is certainly not right.

Stable is nice, because if you mark a bunch of stones as alive/dead, your changes aren't lost.

I worry that it might not be obvious, though, where all of your overrides are in that case? Like, you might mark a stone as dead, and not realize that the score being shown by the app is still the new estimation + your original override, depending on how the territory map changes and where you clicked?

If we erase the overrides every time a stone's status changes, then that is far less error prone, and more obvious to a user what's going on. Users would catch on pretty quickly that they should just mark stones as alive/dead before overriding to begin with, which I think makes sense. It also gives the users an easy way to clear all of their overrides. I think I'm leaning towards this option?

Usually when I'm overriding, its not because I'm arguing the life/death of stones, so much as what I think I can get away with against this opponent xD. So like I'll think: "I agree with the computer that this group will live. But I think I can get another 5 points here. Okay, what if it dies? Well yeah, but I can probably sneak back 3 points here...." In this user story, clearing on each dead/alive change makes more sense to me.

I'm up for implementing it, but I'm currently on vacation, and so I'm not sure if I will be able to implement it before the last week of Jan. I can do it then, or I'm happy for you to take a swing at it, if you'd like to before then!

apetresc commented 7 months ago

I haven't thought about it too deeply just yet, but my first instinct is just to add the dead/alive toggles to the key into the estimateOverrides object, so it's not just a function of the (x,y) coordinates, but rather a function of ((x, y), deadOverrideHash), where deadOverrideHash is some hash of the stones whose life/death status has been overridden by the user.

That way estimateOverrides will store a separate set of overrides for each configuration of dead/alive stones, which I think would be the least surprising way we could handle this. If you mess with the territory/influence estimate, then you change the life+death status of some groups, you have to re-mess with the new estimate. Then if you switch the L+D status back and forth, it keeps the two overrides separately.

Does that sound reasonable?