33
\$\begingroup\$

Description

This is the good old game Memory with a twist: Every time you pick a wrong pair, the two tiles you chose will switch their location. So sometimes you might think that a tile is at one location when in fact... it has moved. And it might feel like you have no idea where it is anymore.

Author of this game is not responsible for any broken keyboards, screens and/or mouse devices.

I am using a semi-Java8-compliant version of GWT. It does not support the Stream API.

Where to play?

The game can be played here: http://www.zomis.net/codereview/memory/MemoryGWT.html

Class Summary

  • MemoryGWT.java, MemoryGWT.html: Main GWT Entry Point.
  • MemoryGWT.css: Just some simple CSS.
  • MemoryBoard.java: The view class used for the main Memory Board
  • FieldView.java: The view for each tile.
  • ListUtils.java: Excluded from the review as it is not my code originally. Simply contains an implementation of shuffle as Collections.shuffle does not work in GWT.

Code

FieldView.java: (39 lines, 760 bytes)

public class FieldView implements IsWidget {

    private static final String HIDDEN_LABEL = "";
    private final Button widget;
    private int value;

    public FieldView(int value) {
        this.value = value;
        widget = new Button(HIDDEN_LABEL);
        widget.setStyleName("game-button", true);
    }

    @Override
    public Button asWidget() {
        return widget;
    }

    public int getValue() {
        return value;
    }

    public void setValue(int value) {
        this.value = value;
    }

    public void showValue() {
        widget.setText(String.valueOf(value));
    }

    public void hideValue() {
        widget.setText(HIDDEN_LABEL);
    }

}

MemoryBoard.java: (86 lines, 2081 bytes)

public class MemoryBoard implements IsWidget {

    private final Grid grid;
    private final Random random = new Random();
    private FieldView previousClicked;
    private boolean timerRunning;

    public MemoryBoard(int width, int height) {
        if ((width * height) % 2 != 0) {
            throw new IllegalArgumentException("width * height must be an even number");
        }
        grid = new Grid(height, width);
        grid.setStyleName("game");
        List<Integer> ints = new ArrayList<>();
        for (int i = 0; i < width * height / 2; i++) {
            ints.add(i);
            ints.add(i);
        }
        ListUtils.shuffle(ints, random);

        for (int x = 0; x < width; x++) {
            for (int y = 0; y < height; y++) {
                int value = ints.remove(ints.size() - 1);
                FieldView view = new FieldView(value);
                view.asWidget().addClickHandler(e -> clicked(view));
                grid.setWidget(y, x, view);
            }
        }

    }

    private void clicked(FieldView view) {
        if (view == previousClicked) {
            return;
        }
        if (timerRunning) {
            return;
        }
        if (previousClicked != null) {
            boolean same = previousClicked.getValue() == view.getValue();
            view.showValue();
            if (!same) {
                Timer timer = new Timer() {
                    @Override
                    public void run() {
                        // switch the two values
                        int previous = previousClicked.getValue();
                        previousClicked.setValue(view.getValue());
                        view.setValue(previous);
                        view.hideValue();
                        previousClicked.hideValue();
                        previousClicked = null;
                        timerRunning = false;
                    }
                };
                timerRunning = true;
                timer.schedule(2000);
            }
            else {
                previousClicked = null;
            }
        }
        else {
            view.showValue();
            previousClicked = view;
        }

    }

    @Override
    public Widget asWidget() {
        return grid;
    }

}

MemoryGWT.java: (15 lines, 345 bytes)

public class MemoryGWT implements EntryPoint {

    @Override
    public void onModuleLoad() {
        final MemoryBoard memory = new MemoryBoard(6, 6);

        RootPanel.get("gameContainer").add(memory);

    }
}

MemoryGWT.css

.game-button {
    width: 42px;
    height: 42px;
}

MemoryGWT.html

<!doctype html>
<!-- The DOCTYPE declaration above will set the     -->
<!-- browser's rendering engine into                -->
<!-- "Standards Mode". Replacing this declaration   -->
<!-- with a "Quirks Mode" doctype is not supported. -->

<html>
  <head>
    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
    <link type="text/css" rel="stylesheet" href="MemoryGWT.css">
    <title>Memory Extreme</title>

    <script type="text/javascript" src="memorygwt/memorygwt.nocache.js"></script>
  </head>

  <body>

    <h1>Memory</h1>
    <p>Good old memory with a twist: When you have picked a non-matching pair, the two tiles you have chosen switch.</p>

    <!-- OPTIONAL: include this if you want history support -->
    <iframe src="javascript:''" id="__gwt_historyFrame" tabIndex='-1' style="position:absolute;width:0;height:0;border:0"></iframe>

    <noscript>
      <div style="width: 22em; position: absolute; left: 50%; margin-left: -11em; color: red; background-color: white; border: 1px solid red; padding: 4px; font-family: sans-serif">
        Your web browser must have JavaScript enabled
        in order for this application to display correctly.
      </div>
    </noscript>

    <div id="gameContainer"></div>
  </body>
</html>

Questions

The main concern I have is: Is it somehow possible to cheat by debugging the Javascript variables to figure out the values of the tiles in advance? I am considering to make some more GWT stuff, but if it is possible to cheat then I will have to alter the way that I am making it (generate stuff dynamically for example, right before you actually make a move).

Other than that: How is my use of GWT?

Other than that: Any comments welcome. I am not very focused on the HTML and CSS stuff though, they are just included for the sake of completeness.

Unobfuscated Javascript

Unobfuscated compiled Javascript is available here: http://www.zomis.net/codereview/memory/memorygwt/

\$\endgroup\$
  • 1
    \$\begingroup\$ Amazing---I created this exact game, with the exact same name, several years ago while learning to program. vivatropolis.org/istvan/MemoryTwist.xhtml \$\endgroup\$ – Lily Chung Oct 19 '14 at 22:44
  • 1
    \$\begingroup\$ Suggesting minor usability improvement. Once you have selected two tiles, you must wait to click on the next two. Clicking on the third tile should hide the selected two and display the thirds. This will speed up game play. \$\endgroup\$ – Drejc Oct 21 '14 at 7:43
11
+100
\$\begingroup\$

YES

The game is cheatable.

Like most software, the game is cheatable whenever there is a debugger available.

In this case, I have loaded up FireBug in Firefox, and learned a few tricks. Here's how the game is cheatable.

Note, this is based on playing the game from your source here:

Process

  1. Enable FireBug
  2. Load the game (click the link)
  3. In the 'Scripts' tab in Firebug, select script[9] and scroll to line 273. This is the JavaScript code used to populate your initial grid....:

    Which Scripts to debug

  4. Enable a break point at about that line

  5. Hit f5 to refresh the page. This should cause the page to partially load, and the break point will be where you set it....
  6. Then, in the "Watch" tab we can inspect the ints array, which is the 'shuffled' collection of cell values that the grid is initialized with. Each member in the array documents the exact value which is placed at each location. This can be used to chose exact pairs without concern for mistakes.

    Cell Values

  7. Profit?

Notes

It took me a little while to track down exactly where the GWT content is stored, about.... 30 minutes to find FireBug, install, learn how it works, track down where things go, understand how GWT loads the scripts from an array of text which it loads separately, etc.

A person who is more familiar with JavaScript, the execution model in a browser, and the debug tooling could probably do it much faster.

The obfuscated code bases would make that harder, but the concept would still be the same.

\$\endgroup\$
13
\$\begingroup\$

Hmm, very nice! Nice exercise for memory (despite the bit, uhm, sadistic twist), and the code is nicely written and very hard to pick on: what can be final is final, arguments are validated, no magic numbers or duplicated string literals, well-formatted and clear, easy to understand. I noticed you pick off the last element from the shuffled list, and even the HTML passes the w3c validator.

As a minor optimization, I would recommend removing the click listeners from the matched tiles that were revealed.

The Random object is used only once, so it could as well be a local variable, or better yet, just embedded in the statement where you use it. Later if you want to make this testable, you can consider adding it as a constructor argument. In any case, in the current code there's no visible reason to make it a member field.

Although it's clever to pop off the elements of ints from the end, how about not removing anything at all:

for (int x = 0; x < width; x++) {
    for (int y = 0; y < height; y++) {
        int value = ints.get(x * width + height);

Although this is more "efficient" in theory, in your use case this is practically negligible. You could as well use the short and sweet and inefficient ints.remove(0), it still wouldn't make a practical difference.

A minor usability note: the numbers on the memory cards are 0-based, on the example page ranging from 0 to 17. That's fine for geeks, but regular human beings would probably expect 1-18 instead.

A simple usability improvement that could make the game much more addictive is adding a count of trials. That way I will know if I'm getting better or not, otherwise it's too hard to tell. Further building on this, a history of the scores of recently completed games would be great too.

\$\endgroup\$
  • 5
    \$\begingroup\$ Your suggestion about removing click listeners from matched tiles is not only a "minor optimization", it solves a bug ;) \$\endgroup\$ – Simon Forsberg Oct 19 '14 at 19:08
  • 2
    \$\begingroup\$ I thing that regular human beings wouldn't expect numbers in a memory game at all, but pretty pictures. So I think that starting with 0 is fine for numbers. \$\endgroup\$ – tim Oct 19 '14 at 19:53
11
\$\begingroup\$

Switch negative if statements

I would switch the if statements around in code like this:

if (!condition) {
    // long block of code
} else {
    // short block of code
}

You have this twice: previousClicked != null and !same.

I would do this for two reasons: it's easier to read if(condition) than if(!condition), and because the shorter code is at the top it also becomes easier to see what else statements finish what if statements when you have nested statements.

Usability

I would add a functionality that hides the two open tiles immediately if the user clicks anywhere, instead of ignoring the click and letting the timer finish. It might just be that I am impatient, but it is quite annoying for me to wait out the timer.

setWidget

grid.setWidget(y, x, view);

Should this be grid.setWidget(x, y, view);?

Is it somehow possible to cheat by debugging the Javascript variables to figure out the values of the tiles in advance?

This would be a lot easier to test with non-obfuscated code. If you change your example, I'm sure someone would try to cheat it.

\$\endgroup\$
  • 1
    \$\begingroup\$ About grid.setWidget: No, it should be like that. (I know, it feels weird...) \$\endgroup\$ – Simon Forsberg Oct 19 '14 at 20:08
  • \$\begingroup\$ As for the obfuscated code: I can provide a non-obfuscated version as well (at a later time), but what I will actually use for any game such as this, is of course obfuscated code. \$\endgroup\$ – Simon Forsberg Oct 19 '14 at 20:11
3
\$\begingroup\$

I am sure that it is possible to find the right Javascript files and use the Cache to recreate the grid of numbers before you actually click any game piece, but seriously, if someone really took the time to cheat on an abnormally hard memory game, and took the time to find the files and then read through them or even built a parser to figure it out.....

Would it really make a difference? are you selling this game? are you going to make money off of this rendition of the game?

If you want it fool proof then you should use server side variables to hold the grid of numbers to keep is secret unless you "turn over" a game piece to see what it is.


I would make some changes to this code

private void clicked(FieldView view) {
    if (view == previousClicked) {
        return;
    }
    if (timerRunning) {
        return;
    }
    if (previousClicked != null) {
        boolean same = previousClicked.getValue() == view.getValue();
        view.showValue();
        if (!same) {
            Timer timer = new Timer() {
                @Override
                public void run() {
                    // switch the two values
                    int previous = previousClicked.getValue();
                    previousClicked.setValue(view.getValue());
                    view.setValue(previous);
                    view.hideValue();
                    previousClicked.hideValue();
                    previousClicked = null;
                    timerRunning = false;
                }
            };
            timerRunning = true;
            timer.schedule(2000);
        }
        else {
            previousClicked = null;
        }
    }
    else {
        view.showValue();
        previousClicked = view;
    }

}

I would combine the first two if statements using an "or" operator, and then move the guard clause to the if statement getting rid of the negation.

After I got rid of the negation it is clear that I don't need the boolean variable same because it is only used once and thrown away, so I just throw the equality into the if statement.

This leaves us with:

private void clicked(FieldView view) {
    if (view == previousClicked || timerRunning) {
        return;
    }
    if (previousClicked != null) {
        boolean same = previousClicked.getValue() == view.getValue();
        view.showValue();
        if (previousClicked.getValue() == view.getValue()) {
            previousClicked = null;
        } else {
            Timer timer = new Timer() {
                @Override
                public void run() {
                    // switch the two values
                    int previous = previousClicked.getValue();
                    previousClicked.setValue(view.getValue());
                    view.setValue(previous);
                    view.hideValue();
                    previousClicked.hideValue();
                    previousClicked = null;
                    timerRunning = false;
                }
            };
            timerRunning = true;
            timer.schedule(2000);
        }
    } else {
        view.showValue();
        previousClicked = view;
    }
}

I also used proper Bracing around the if/else statements. I like doing it like that and sometimes wish that C# standards would let me do it the same way. (shh don't tell anyone I said that)

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for your answer, but your answer doesn't touch anything at all that I started the bounty for: I am still looking for an answer about whether or not the game is "cheatable" by debugging the Javascript variabels \$\endgroup\$ – Simon Forsberg Dec 17 '14 at 15:48
  • \$\begingroup\$ No, I am not selling this rendition of the game. But I am considering rewriting my Minesweeper game with GWT - that is why I am wondering if it is cheatable or not. I am aware that using server-sided is the only way to make it perfectly uncheatable, I am using a server for Minesweeper, but I like to have an independent client-only version as well. \$\endgroup\$ – Simon Forsberg Dec 18 '14 at 15:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.