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

possible logic error in 60_mastermind #611

Closed
AnthonyMichaelTDM opened this issue Mar 7, 2022 · 10 comments
Closed

possible logic error in 60_mastermind #611

AnthonyMichaelTDM opened this issue Mar 7, 2022 · 10 comments

Comments

@AnthonyMichaelTDM
Copy link
Contributor

AnthonyMichaelTDM commented Mar 7, 2022

while porting 60_mastermind to rust, I came across a peculiar issue, here in mastermind.bas (the flawed logic is present in every other port I looked at too):

1010 FOR X=1 TO P
1020 GOSUB 3500
1030 IF I(X)=0 THEN 1070
1035 GOSUB 6500
1040 GOSUB 4000
1050 GOSUB 4500
1060 IF B1<>B OR W1<>W THEN I(X)=0
1070 NEXT

this code basically looks through every possible combination,
for all that haven't already been marked as impossible (with previously given player feedback),
it checks whether or not the black and white pins that that combination should get are not-equal to what the previous guess got ... if they are equal, the combination is marked as possible, if they aren't equal then the combination is marked as impossible

the issue is, this almost always will mark the correct answer as impossible, unless its first guess happens to be right

a potential fix (that I can't test but did work in my rust port), is changing that logic from 1060 IF B1<>B OR W1<>W THEN I(X)=0 to 1060 IF B1>B OR W1>W THEN I(X)=0 which, as compared to 1060 IF B1>B OR W1>W THEN I(X)=0, makes the computer a bit more powerful, and actually able to win with some consistency

note: it could very well be that I messed something up in my implementation, hence why I'm making this issue, in case people agree that this is a problem I'll be making a PR to fix it that I'll link to in the comments

edit: typo

@AnthonyMichaelTDM AnthonyMichaelTDM changed the title possible logic error in 66_mastermind possible logic error in 60_mastermind Mar 7, 2022
@AnthonyMichaelTDM
Copy link
Contributor Author

as promised #613

@coding-horror
Copy link
Owner

This is great -- let's be sure it is in the readme.md for the project as well!

@AnthonyMichaelTDM
Copy link
Contributor Author

I'll write something up later today

@AnthonyMichaelTDM
Copy link
Contributor Author

AnthonyMichaelTDM commented Mar 7, 2022

how should I go about doing that @coding-horror, there seem to be multiple precedents:

  • a note saying when and what was patched (as in 02_amazing)
    or
  • revert the change to the source and add a note detailing what the issues is and how future implementers should fix it (as in 84_super_star_trek)

@coding-horror
Copy link
Owner

Mainly I think the note should be in the readme.md in the game folder under implementation notes heading.. that seems like the best place?

@AnthonyMichaelTDM
Copy link
Contributor Author

Right, but which would you prefer, documenting what was fixed, or saying what should be fixed

@coding-horror
Copy link
Owner

both, if you have time! 🙏

@AnthonyMichaelTDM
Copy link
Contributor Author

sure, does that ^ look good?

@AnthonyMichaelTDM
Copy link
Contributor Author

well i think that settles it, issue is fixed so no use keeping this issue open

@jnellis
Copy link
Contributor

jnellis commented Apr 1, 2022

well i think that settles it, issue is fixed so no use keeping this issue open

I am unconvinced. Here is a 2 color, 2 position game, which is the simplest game that isn't just black or white as a possibility.

Here, with this fix, the computer guessed that same guess twice.

NOW I GUESS.  THINK OF A COMBINATION.
HIT RETURN WHEN READY:?
MY GUESS IS: WB  BLACKS, WHITES ? 1,0
MY GUESS IS: BB  BLACKS, WHITES ? 0,0
MY GUESS IS: WB  BLACKS, WHITES ? 1,0
MY GUESS IS: WW  BLACKS, WHITES ? 2,0
I GOT IT IN  4  MOVES!

I don't see how you were witnessing it throwing up the "you gave inconsistent results" every time unless you had the reporting of black and white pegs backwards.

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

No branches or pull requests

3 participants