0
\$\begingroup\$

Features

The repository is available online. The game engine is nearly perfect. I would like a review of the UI code and the Android specific stuff.

package dev.game.adventure;

import android.annotation.SuppressLint;
import android.content.Context;
import android.graphics.*;
import android.graphics.drawable.Drawable;
import android.media.Image;
import android.support.annotation.Nullable;
import android.support.v4.view.MotionEventCompat;
import android.support.v7.app.*;
import android.os.*;
import android.text.method.ScrollingMovementMethod;
import android.util.AttributeSet;
import android.view.*;
import android.widget.*;

import java.net.MalformedURLException;
import java.net.URL;
import java.util.Collection;
import java.util.Iterator;
import java.util.Random;

/**
 * An example full-screen activity that shows and hides the system UI (i.e.
 * status bar and navigation/system bar) with user interaction.
 */
public class FullscreenActivity extends AppCompatActivity {
    /**
     * Whether or not the system UI should be auto-hidden after
     * {@link #AUTO_HIDE_DELAY_MILLIS} milliseconds.
     */
    private static final boolean AUTO_HIDE = true;
    AdventureGame ag;
    public TextView textView;
    public LinearLayout mainLayout;

    /**
     * If {@link #AUTO_HIDE} is set, the number of milliseconds to wait after
     * user interaction before hiding the system UI.
     */
    private static final int AUTO_HIDE_DELAY_MILLIS = 3000;

    /**
     * Some older devices needs a small delay between UI widget updates
     * and a change of the status and navigation bar.
     */
    private static final int UI_ANIMATION_DELAY = 300;
    private final Handler mHideHandler = new Handler();
    private View mContentView;
    private final Runnable mHidePart2Runnable = new Runnable() {
        @SuppressLint("InlinedApi")
        @Override
        public void run() {
            // Delayed removal of status and navigation bar

            // Note that some of these constants are new as of API 16 (Jelly Bean)
            // and API 19 (KitKat). It is safe to use them, as they are inlined
            // at compile-time and do nothing on earlier devices.
            mContentView.setSystemUiVisibility(View.SYSTEM_UI_FLAG_LOW_PROFILE
                    | View.SYSTEM_UI_FLAG_FULLSCREEN
                    | View.SYSTEM_UI_FLAG_LAYOUT_STABLE
                    | View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY
                    | View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION
                    | View.SYSTEM_UI_FLAG_HIDE_NAVIGATION);
        }
    };
    private View mControlsView;
    private final Runnable mShowPart2Runnable = new Runnable() {
        @Override
        public void run() {
            // Delayed display of UI elements
            ActionBar actionBar = getSupportActionBar();
            if (actionBar != null) {
                actionBar.show();
            }
            mControlsView.setVisibility(View.VISIBLE);
        }
    };
    private boolean mVisible;
    private final Runnable mHideRunnable = new Runnable() {
        @Override
        public void run() {
            hide();
        }
    };
    /**
     * Touch listener to use for in-layout UI controls to delay hiding the
     * system UI. This is to prevent the jarring behavior of controls going away
     * while interacting with activity UI.
     */
    private final View.OnTouchListener mDelayHideTouchListener = new View.OnTouchListener() {
        @Override
        public boolean onTouch(View view, MotionEvent motionEvent) {
            if (AUTO_HIDE) {
                delayedHide(AUTO_HIDE_DELAY_MILLIS);
            }
            return false;
        }
    };

    private Player player = null;

    private static final long serialVersionUID = 100L;

    // Load an image from the net, making sure it has already been
    // loaded when the method returns
    public Image loadPicture(String imageName) {
        Image im = null;
        // Load the image from the net
        return im;
    }

    // Load and play a sound from /usr/local/hacks/sounds/
    public void playSound(String name) {
        URL u = null;

        try {
            u = new URL("http://www.student.nada.kth.se/~d99-nro/sounds/" + name + ".au");
        } catch (MalformedURLException e) {
            e.printStackTrace();
        }

//        AudioClip a = Applet.newAudioClip(u);
        //      a.play();
    }

    private ScrollView vScroll;
    TextView scrollable;
    TableLayout tbl2;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_fullscreen);
        scrollable = (TextView) findViewById(R.id.textView1);
        scrollable.setMovementMethod(new ScrollingMovementMethod());
        ag = findViewById(R.id.view);
        ag.setBackground(getResources().getDrawable(R.mipmap.dungeon));
        ag.text = scrollable;
        ag = findViewById(R.id.view);
        new Adventure(scrollable, this, ag);
        Button buttonOne = (Button) findViewById(R.id.close);
        buttonOne.setOnClickListener(new Button.OnClickListener() {
            public void onClick(View v) {
                ag.mainCharacter.go("North", ag.target);
                ag.display("North", ag.target, ag.mainCharacter.place);
            }
        });

        Button buttonOne2 = (Button) findViewById(R.id.close2);
        buttonOne2.setOnClickListener(new Button.OnClickListener() {
            public void onClick(View v) {
                ag.mainCharacter.go("East", ag.target);
                ag.display("East", ag.target, ag.mainCharacter.place);
            }
        });

        Button buttonOne3 = (Button) findViewById(R.id.close3);
        buttonOne3.setOnClickListener(new Button.OnClickListener() {
            public void onClick(View v) {
                ag.mainCharacter.go("South", ag.target);
                ag.display("South", ag.target, ag.mainCharacter.place);
            }
        });

        Button buttonOne4 = (Button) findViewById(R.id.close4);
        buttonOne4.setOnClickListener(new Button.OnClickListener() {
            public void onClick(View v) {
                ag.mainCharacter.go("West", ag.target);
                ag.display("West", ag.target, ag.mainCharacter.place);
            }
        });

        Button buttonOne5 = (Button) findViewById(R.id.close5);
        buttonOne5.setOnClickListener(new Button.OnClickListener() {
            public void onClick(View v) {
                ag.mainCharacter.go("Up", ag.target);
                ag.display("Up", ag.target, ag.mainCharacter.place);
            }
        });

        Button buttonOne6 = (Button) findViewById(R.id.close6);
        buttonOne6.setOnClickListener(new Button.OnClickListener() {
            public void onClick(View v) {
                ag.mainCharacter.go("Down", ag.target);
                ag.display("Down", ag.target, ag.mainCharacter.place);
            }
        });
        ag.mainCharacter.goTo("Dungeon2", this);
    }

    private OnScoreListener onScoreListener = new OnScoreListener() {
        @Override
        public void onScore(int score) {
            textView.setText("Score: " + score);
        }
    };

    @Override
    protected void onPostCreate(Bundle savedInstanceState) {
        super.onPostCreate(savedInstanceState);

        // Trigger the initial hide() shortly after the activity has been
        // created, to briefly hint to the user that UI controls
        // are available.
        // delayedHide(100);
    }

    private void toggle() {
        if (mVisible) {
            hide();
        } else {
            show();
        }
    }

    private void hide() {
        // Hide UI first
        ActionBar actionBar = getSupportActionBar();
        if (actionBar != null) {
            actionBar.hide();
        }
        mControlsView.setVisibility(View.GONE);
        mVisible = false;

        // Schedule a runnable to remove the status and navigation bar after a delay
        mHideHandler.removeCallbacks(mShowPart2Runnable);
        mHideHandler.postDelayed(mHidePart2Runnable, UI_ANIMATION_DELAY);
    }

    @SuppressLint("InlinedApi")
    private void show() {
        // Show the system bar
        mContentView.setSystemUiVisibility(View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN
                | View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION);
        mVisible = true;

        // Schedule a runnable to display UI elements after a delay
        mHideHandler.removeCallbacks(mHidePart2Runnable);
        mHideHandler.postDelayed(mShowPart2Runnable, UI_ANIMATION_DELAY);
    }

    /**
     * Schedules a call to hide() in delay milliseconds, canceling any
     * previously scheduled calls.
     */
    private void delayedHide(int delayMillis) {
        mHideHandler.removeCallbacks(mHideRunnable);
        mHideHandler.postDelayed(mHideRunnable, delayMillis);
    }
}
interface OnScoreListener {
    void onScore(int score);
}
class AdventureGame extends View {
    public AdventureGame(Context context, @Nullable AttributeSet attrs) {
        super(context, attrs);

        NBRSTEPS = 4;//difficulty;
        heroName = "Jamie";//name;
        target = (FullscreenActivity) context;
        this.onScoreListener = onScoreListener;
        this.cont = context;
        x = new int[NBRSTEPS];
        y = new int[NBRSTEPS];
        this.target = target;
        hero_positions = new int[NBRSTEPS];
        int resourceIdFalling = 0;
        int resourceIdHero = 0;
        if (heroName.equals("Jamie")) {
            resourceIdFalling = R.mipmap.jamie;
            resourceIdHero = R.mipmap.jamie;
            setBackground(getResources().getDrawable(R.mipmap.view1));
        }
        if (heroName.equals("Spaceship")) {
            resourceIdFalling = R.mipmap.jamie;
            resourceIdHero = R.mipmap.jamie;
            setBackground(getResources().getDrawable(R.mipmap.view1));
        }
        falling = BitmapFactory.decodeResource(getResources(), resourceIdFalling); //load a falling image
        hero = BitmapFactory.decodeResource(getResources(), resourceIdHero); //load a hero image
        ballW = falling.getWidth();
        ballH = falling.getHeight();
    }

    public Collection<Person> getPersons() {
        return persons;
    }

    Person mainCharacter;

    public void setPersons(Collection<Person> persons) {
        this.persons = persons;
    }

    public void display(String s1, FullscreenActivity target, Place p) {

        setBackground(p.image); // FIXME: All places have the same background
    }

    /**
     * Persons at this Place.
     */
    public Collection<Person> persons;
    int NBRSTEPS; // number of discrete positions in the x-dimension; must be uneven
    String heroName;
    int screenW;
    int screenH;
    int[] x; // x-coordinates for falling objects
    int[] y; // y-coordinates for falling objects
    int[] hero_positions; // x-coordinates for hero
    Random random = new Random();
    int ballW; // width of each falling object
    int ballH; // height of ditto
    float dY; //vertical speed
    Bitmap falling, hero;
    int heroXCoord;
    int heroYCoord;
    int xsteps;
    int score;
    int offset;
    boolean gameOver; // default value is false
    boolean toastDisplayed;
    boolean paused = false;
    OnScoreListener onScoreListener;
    Context cont;
    TextView text;
    FullscreenActivity target;

    // constructor, load images and get sizes
    public AdventureGame(TableLayout layout,

                         TextView textView,
                         Context context, int difficulty, String name, OnScoreListener onScoreListener) {
        super(context);
        NBRSTEPS = difficulty;
        heroName = name;
        target = (FullscreenActivity) context;
        this.onScoreListener = onScoreListener;
        this.cont = context;
        x = new int[NBRSTEPS];
        y = new int[NBRSTEPS];
        this.target = target;
        hero_positions = new int[NBRSTEPS];
        int resourceIdFalling = 0;
        int resourceIdHero = 0;
        if (heroName.equals("Jamie")) {
            resourceIdFalling = R.mipmap.jamie;
            resourceIdHero = R.mipmap.jamie;
            setBackground(getResources().getDrawable(R.mipmap.view1));
        }
        if (heroName.equals("Spaceship")) {
            resourceIdFalling = R.mipmap.jamie;
            resourceIdHero = R.mipmap.jamie;
            setBackground(getResources().getDrawable(R.mipmap.view1));
        }
        falling = BitmapFactory.decodeResource(getResources(), resourceIdFalling); //load a falling image
        hero = BitmapFactory.decodeResource(getResources(), resourceIdHero); //load a hero image
        ballW = falling.getWidth();
        ballH = falling.getHeight();

        textView.setVisibility(View.VISIBLE);
        textView.setText("Hello world");
        this.text = textView;
    }


    // set coordinates, etc.
    void initialize() {
        if (!gameOver) { // run only once, when the game is first started
            int maxOffset = (NBRSTEPS - 1) / 2;
            for (int i = 0; i < x.length; i++) {
                int origin = (screenW / 2) + xsteps * (i - maxOffset);
                x[i] = origin - (ballW / 2);
                hero_positions[i] = origin - hero.getWidth();
            }
            int heroWidth = hero.getWidth();
            int heroHeight = hero.getHeight();

            hero = Bitmap.createScaledBitmap(hero, heroWidth * 2, heroHeight * 2, true);
            heroYCoord = screenH - 2 * heroHeight; // bottom of screen

        }
        for (int i = 0; i < y.length; i++) {
            y[i] = -random.nextInt(1000); // place items randomly in vertical direction
        }

        offset = (NBRSTEPS - 1) / 2; // place hero at centre of the screen
        heroXCoord = hero_positions[offset];

        // initialize or reset global attributes
        dY = 2.0f;
        score = 0;
        gameOver = false;
        toastDisplayed = false;
    }

    // method called when the screen opens
    @Override
    public void onSizeChanged(int w, int h, int oldw, int oldh) {
        super.onSizeChanged(w, h, oldw, oldh);
        screenW = w;
        screenH = h;
        xsteps = w / NBRSTEPS;
        initialize();
    }

    // method called when the "game over" toast has finished displaying
    void restart(Canvas canvas) {

        toastDisplayed = true;

        initialize();
        draw(canvas);
    }

    //Bitmap hero;
    // update the canvas in order to display the game action
    @Override
    public void onDraw(Canvas canvas) {
        if (toastDisplayed) {
            restart(canvas);
            return;
        }
        super.onDraw(canvas);

        //System.out.println("1 drawing the place:"+name);
        //g.drawImage(image, 0, 20, this);
        int xx = 200;
        int yy = 0;
        if (persons != null) {
            synchronized (persons) {
                Iterator<Person> iterate = persons.iterator();
                while (iterate.hasNext()) {
                    Person p = iterate.next();
                    System.out.println("Fullscreen drawing:" + p.getName());
                   // Drawable appearance = getResources().getDrawable(p.image);
                    if (p.getImage() != 0) {
                        hero = BitmapFactory.decodeResource(getResources(), p.getImage()); //load a hero image

                        //hero = Bitmap.createScaledBitmap(hero, 10 * 2, 10 * 2, true);

                        //if (appearance != null) {
                        // Draw the visible person's appearance
                        //  g.drawImage(appearance, xx,
                        //          240 + yy - (appearance.getHeight(this) * 4) / 5,
                        //          this);
                        canvas.drawBitmap(hero, xx, 0, null); //Draw the hero on the canvas.

                        //canvas.restore();
                        //Call the next frame.
                        //  invalidate();

                        // Draw the name
                        Paint paint = new Paint();
                        paint.setStyle(Paint.Style.FILL);

                        canvas.save();
                        canvas.translate(100, 200);

                        // make the entire canvas white
//                        canvas.drawColor(Color.WHITE);

                        // draw some text using STROKE style
                      //  paint.setStyle(Paint.Style.STROKE);
                        paint.setStrokeWidth(1);
                        paint.setColor(Color.BLACK);
                        paint.setTextSize(50);
                        canvas.drawText(p.name, 0, 0, paint);

        /*          g.setColor(Color.white);
                    g.drawString(p.name, xx + (appearance.getWidth(this)) / 2,
                            240 + yy - (appearance.getHeight(this) * 4) / 5);
                    g.setColor(Color.black);
        */        //}
                        xx += 100;
                        yy = (int) (Math.random() * 20);
                    }
                }
            }
        }
        //this.text.setVisibility(View.VISIBLE);
        //this.text.setText("Hello world");
        canvas.save(); //Save the position of the canvas.
        canvas.restore();
        //Call the next frame.
        invalidate();
    }

    // event listener for when the user touches the screen
    @Override
    public boolean onTouchEvent(MotionEvent event) {
        if (paused) {
            paused = false;
        }
        int action = MotionEventCompat.getActionMasked(event);
        if (action != MotionEvent.ACTION_DOWN || gameOver) { // non-touchdown event or gameover
            return true; // do nothing
        }
        int coordX = (int) event.getX();
        int xCentre = (screenW / 2) - (hero.getWidth() / 2);
        int maxOffset = hero_positions.length - 1; // can't move outside right edge of screen
        int minOffset = 0; // ditto left edge of screen
        if (coordX < xCentre && offset > minOffset) { // touch event left of the centre of screen
            offset--; // move hero to the left
        }
        if (coordX > xCentre && offset < maxOffset) { // touch event right of the centre of screen
            offset++; // move hero to the right
        }
        heroXCoord = hero_positions[offset];
        return true;
    }
}

Layout

<TableLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:id="@+id/tableLayout1"
    android:layout_width="fill_parent"
    android:layout_height="fill_parent"
    android:padding="10dip">

    <TableRow
        android:id="@+id/tableRow1"
        android:layout_width="fill_parent"
        android:layout_height="wrap_content">

        <!-- Some stuff goes here -->

        />

        <Button
            android:id="@+id/close"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_column="0"
            android:text="@string/north" />

        <Button
            android:id="@+id/close2"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_column="1"
            android:text="@string/east" />

        <Button
            android:id="@+id/close3"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_column="2"
            android:text="@string/south" />

        <Button
            android:id="@+id/close4"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_column="3"
            android:text="@string/west" />

        <Button
            android:id="@+id/close5"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_column="4"
            android:text="@string/up" />

        <Button
            android:id="@+id/close6"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:layout_alignParentBottom="true"
            android:layout_column="5"
            android:text="@string/down" />

    </TableRow>

    <TableRow
        android:id="@+id/tableRow2"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content">

        <ScrollView
            android:layout_width="fill_parent"
            android:layout_height="fill_parent"
            android:layout_span="6"
            android:layout_weight="1"
            android:scrollbars="none">

            <!-- Some stuff goes here -->

            />

            <TextView
                android:id="@+id/textView1"
                android:layout_width="wrap_content"
                android:layout_height="wrap_content"
                android:layout_alignParentLeft="true"
                android:layout_alignParentTop="true"

                android:background="#FFFFFF"
                android:gravity="bottom"
                android:maxLines="2"
                android:scrollbars="vertical"
                android:textAppearance="?android:attr/textAppearanceLarge"
                android:textColor="#000000" />

        </ScrollView>

    </TableRow>

    <TableRow
        android:id="@+id/tableRow3"
        android:layout_width="fill_parent"
        android:layout_height="fill_parent"
        android:layout_weight="1">

        <!-- Some stuff goes here -->

        />

        <dev.game.adventure.AdventureGame
            android:id="@+id/view"
            android:layout_width="fill_parent"
            android:layout_height="fill_parent"
            android:layout_span="6"
            android:layout_weight="1"
            android:adjustViewBounds="true"
            android:background="#FFFFFF"
            android:paddingLeft="10dp"
            android:paddingRight="10dp"
            android:paddingTop="10dp" />
    </TableRow>


</TableLayout>

Screenshot

enter image description here

\$\endgroup\$
  • 1
    \$\begingroup\$ "which works for basically any game" - any adventure-style game, that is. A quite specific type of adventure-style games. I took a quick look at your game engine and I'd strongly recommend to decouple it from your GUI. I'd recommend to separate your model from your view. Your view can know about the model (it has to know what to render), but your model should not know about the view. Imagine that you would want to port your application to make it a Desktop app as well and re-use your code, how would you do that if you refer to android.widget.LinearLayout in your World class? \$\endgroup\$ – Simon Forsberg Nov 21 '17 at 21:56
2
\$\begingroup\$

Class FullscreenActivity

In general:

  • That's a very bad name for a type. I only know it's Fullscreen.
  • The ordering of the variables / constants is horrible. Private static, then protected, then public, private static again, public methods, then variables again.

The code

  • Use fully qualified imports.
  • private static final boolean AUTO_HIDE = true; -> This is always true, so there's no use to even check this flag
  • AdventureGame ag; Why is this not private?
  • Why is textView and mainLayout public?
  • AUTO_HIDE_DELAY_MILLIS and UI_ANIMATION_DELAY remove the comment and give the constant a proper name. Developer don't want to scroll up to read the comment, to understand what's going on 100 lines below.
  • mHideHandler: Use a better name. In general: Don't use abbreviations.
  • mContentView: Use a better name (what's with that m prefix anway?)
  • private static final long serialVersionUID = 100L;: Uh, we tend to remove the compiler warning in our IDE and use a serialVersionUID if it's really needed (like almost never). That's just clutter.
  • loadPicture(String imageName) method: Pretty sure there should be another class responsible for loading images. Also: Use URL, so the comment about "from the net" will be obsolete
  • playSound method: Your comment stantes "load and play a sound", but the method is called playSound. First: If another developer calls it, it might be crucial to know, that the sound is loaded. Second: It does two things anway, a method should do one thing.
  • playSound: Don't e.printStackTrace(). And don't leave commented code. You have a scm, if old code is needed, you still have it. If it's still in the actual code, nobody knows if it's important and has to check the history of the code to know.
  • onCreate: Way too long, a lot of duplication and bad naming. You declare the buttons using "one", "two", and so on, even though you could name them "buttonNorth" and "buttonSouth", etc. Make a method 'createButton' and pass the necessary variables. Also, what's "ag"? What's "R.id.close2"? And this: ag.mainCharacter.goTo("Dungeon2", this). You pass to the mainCharacter this which looks like a circular dependency and must be avoided.
  • toggle() method: Toggle of what?
  • hide() method: Hide what?
  • "// Hide UI first". Well, why don't you write a method "hideUI" and move the responsible code into the method? Same for the other comment: "// Schedule a runnable to remove the status and navigation bar after a delay".
  • show() method: Show what?
  • delayedHide: Method names should have a verb in it, because it does something.

Class AdventureGame

This type is even longer!

  • NBRSTEPS = 4;//difficulty;: Why don't you call it difficulty?
  • heroName = "Jamie";//name;: That's the same as i++; // increment i.
  • this assignments: That's usually done after the super call.
  • x = new int[NBRSTEPS];: Well, x is an array, so it's not a position, but what is it? Again: Proper naming.
  • if (heroName.equals("Jamie")) does the same as if (heroName.equals("Spaceship")). And it should be and if/else-if statement, if the first if condition is true, the second if condition won't, exception you change the heroName within the first if condition.
  • falling = BitmapFactory.decodeResource(getResources(), resourceIdFalling); //load a falling image: Falling is not understood. And: The code for declaring falling and hero is duplicated and can be done in a separate method.
  • public void display() method: Display what on where?
  • public Collection<Person> persons; Even though you have getter and setter, you declare it public.
  • int screenW;: Again: No abbreviations. And I'm pretty sure there's somewhere a Dimension type.
  • int[] hero_positions; // x-coordinates for hero: Use a type for storing Coordinates, instead of an array, that's always painful to work with.
  • Bitmap falling, hero; Use separate declarations. (And use proper naming of course)
  • Oh for ffs, you have two constructors. After one third of the class. Constructors belong to the top. And use constructor overloading, 90% is duplicated code.
  • void initialize(): Why is this not private?
  • if (!gameOver) { // run only once, when the game is first started Don't use a flag which is used for something totally different. Also the code doesn't really match the comment.
  • int maxOffset = (NBRSTEPS - 1) / 2; Max offset of what from where? Why is it divided by two? Why NBRSTEPS (whatever this is) minues one?
  • for (int i = 0; i < x.length; i++) { What does this loop do? It's a total pain to understand, and it's only three lines.
  • y[i] = -random.nextInt(1000);: What's y again? Why minus? What's 1000?
  • heroXCoord = hero_positions[offset];: What does a hero have more positions? Why are you getting the position out of the offset?
  • // initialize or reset global attributes: Why "or"? It always does the same... ?
  • dY = 2.0f;: What's dY again? Why 2.0f?
  • // method called when the screen opens: Well the method is called "onSizeChanges", someone is lying.
  • int xx = 200: XX?? YY?? Why XX = 200? Why yy= 0?
  • Iterator<Person> iterate = persons.iterator();: Use a for each loop
  • System.out.println("Fullscreen drawing:" + p.getName());: Use a logging api.
  • if (p.getImage() != 0) {: Looks like getImage returns an integer, that doesn't look right
  • xx += 100; Why plus hundred?
  • yy = (int) (Math.random() * 20); Why random * 20? Why are you using Math's random and not the "Random random"?
  • canves.save(); canves.restore(): That looks awfully like a hack.

General

Write test cases for your code. Lots of your code isn't even testable, which is usually the case if the design of the application is bad. And in this case: No design at all.

Decouple presentation code from your game code, lots of the code is not reusable and not android specific.

Use any presentation design pattern to organize your presentation logic.

And: "The game engine is nearly perfect." - Well, well, well. Not even the Unreal engine is nearly perfect, so that's quite the bold statement, isn't it? ;-)

\$\endgroup\$
  • \$\begingroup\$ The m-prefix is a common convention when writing Android code to indicate member-variables. I'm not a big fan of that convention myself. \$\endgroup\$ – Simon Forsberg Nov 25 '17 at 12:03
  • \$\begingroup\$ That this.onScoreListener = onScoreListener; statement in one of the constructors seems like it does absolutely nothing. \$\endgroup\$ – Simon Forsberg Nov 25 '17 at 12:14

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.