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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RobertChrist
Copy link

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].

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
Copy link
Member

apetresc commented Jan 4, 2024

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:

Recording.2024-01-03.223759.mp4

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
Copy link
Author

RobertChrist commented Jan 5, 2024

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
Copy link
Member

apetresc commented Jan 6, 2024

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?

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

Successfully merging this pull request may close these issues.

None yet

2 participants