Android Umsetzung eines Spiels mit Minispielen

[Robat]

Hallo liebe Java-Forum-Community,

seit ca. einem Monat sitze ich nun an meinem ersten "größeren" Android-Game. Das Grundprinzip ist an Pou angelehnt. Man hat einen Character den man hochleveln kann, in dem man Minispiele spielt. Mit dem eigentilichen Coden komme ich auch gut klar, aber mir stellt sich eine Frage:
Der Code des MainThreads ist ja für alle Minispiele gleich.

Spoiler: CODE

public class MainThread extends Thread
{

    ////////////////////////////////
    // Constances
    ////////////////////////////////
    public static final int MAX_FPS = 30;

    private static final String TITLE = "MainThread";

    ////////////////////////////////
    // Fields
    ////////////////////////////////
    public static Canvas canvas;

    private double avarageFPS;

    private boolean running;

    private SurfaceHolder holder;

    private GameView view; // kann sich ändern

    ////////////////////////////////
    // Constructor
    ////////////////////////////////
    public MainThread(SurfaceHolder holder, GameView view)
    {
        super();

        this.holder      = holder;
        this.view        = view;

    }


    ////////////////////////////////
    // Overriden Methodes
    ////////////////////////////////


    /**
     * If this thread was constructed using a separate
     * <code>Runnable</code> run object, then that
     * <code>Runnable</code> object's <code>run</code> method is called;
     * otherwise, this method does nothing and returns.
     * <p>
     * Subclasses of <code>Thread</code> should override this method.
     *
     * @see #start()
     * @see #stop()
     */
    @Override
    public void run()
    {
        // temporary vars
        long startTime = 0;
        long timeMillis = 1000/MAX_FPS;
        long waitTime = 0;
        long totalTime = 0;
        long targetTime = 1000/MAX_FPS;

        int frameCount = 0;

        while(running)
        {
            startTime = System.nanoTime();
            canvas    = null;

            try
            {
                canvas = this.holder.lockCanvas();

                synchronized (holder)
                {
                    this.view.update();
                    this.view.draw(canvas);
                }
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
            finally
            {
                if (canvas != null)
                {
                    try
                    {
                        holder.unlockCanvasAndPost(canvas);
                    }
                    catch (Exception e)
                    {
                        e.printStackTrace();
                    }
                }
            }
            timeMillis = (System.nanoTime() - startTime) / 1000000;
            waitTime = targetTime - timeMillis;
            try
            {
                if (waitTime > 0)
                {
                    this.sleep(waitTime);
                }
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
            totalTime += System.nanoTime() - startTime;
            frameCount++;
            if (frameCount == MAX_FPS)
            {
                avarageFPS = 1000/((totalTime/frameCount)/1000000);
                frameCount = 0;
                totalTime = 0;
                Log.d("test", String.valueOf(avarageFPS));
                System.out.println(avarageFPS);
            }
        }
    }


    ////////////////////////////////
    // Getters and Setters
    ////////////////////////////////
    public void setRunning(boolean running)
    {
        this.running = running;
    }

    public double getAvarageFPS()
    {
        return avarageFPS;
    }
}

Jetzt meine Frage:
Wir sind uns sicherlich alle einig, dass wenn man das für 10 Minispiele macht viel zu viele Dateien entstehen. Daher würde ich das ganze gerne abstracter machen. Jedes Minispiel extends von SurfaceView. Daher habe ich schon probiert eine Instance von Surfaceview im Constructor zu übergeben und dann mit einer switch-case zu schauen welches Game nun ausgeführt werden soll aber die idee habe ich sehr schnell wieder verworfen.

Jetzt meine Frage: War der Ansatz richtig? Sollte man das überhaupt "abstracter" machen? Hat jeden vielleicht einen neuen Ansatz für mich?

Danke schon mal im vorraus.

Grüße Robat

 

[Joose]

Dein Ansatz war meiner Meinung nach schon richtig: Es gibt eine Basisklasse von der jedes Minispiel ableitet.
Wofür brauchst du deinere Meinung nach denn dann noch eine switch-Anweisung?

Anmerkung zum Code: Du solltest nicht von Thread ableiten, implementiere stattdessen das Runnable Interface.

 

[Robat]

Hallo Joose,

danke dir für deine schnelle Antwort. Du hast mich damit auf eine Idee gebracht und ich habe gerade festgestellt, dass die Frage von mir irgendwie dämlich war
Was ich jetzt letztendlich gemacht habe ist folgendes:

Jedes Minispiel erbt von einer abstract Klasse GameView die von SurfaceView erbt. In dieser Klasse initialisiere ich die Dinge, die jedes Spiel gleich hat, zB eben den MainThread.

Spoiler: GameView

public abstract class GameView extends SurfaceView implements SurfaceHolder.Callback
{


    ////////////////////////////////
    // Constances
    ////////////////////////////////
    private static final String TITLE = "GameView";

    ////////////////////////////////
    // Fields
    ////////////////////////////////

    private MainThread thread;

    ////////////////////////////////
    // Constructor
    ////////////////////////////////
    public GameView(Context context)
    {
        super(context);

        init();

        getHolder().addCallback(this);
        setFocusable(true);
    }


    ////////////////////////////////
    // Methodes
    ////////////////////////////////

    /**
     * All the update stuff goes here.
     */
    public abstract void update();

    /**
     * Initialization goes here.
     */
    private void init()
    {
        thread = new MainThread(getHolder(), this);
    }


    ////////////////////////////////
    // Overriden Methodes
    ////////////////////////////////

    /**
     * This is called immediately after the surface is first created.
     * Implementations of this should start up whatever rendering code
     * they desire.  Note that only one thread can ever draw into
     * a {@link Surface}, so you should not draw into the Surface here
     * if your normal rendering will be in another thread.
     *
     * @param holder The SurfaceHolder whose surface is being created.
     */
    @Override
    public void surfaceCreated(SurfaceHolder holder)
    {
        thread = new MainThread(getHolder(), this);

        thread.setRunning(true);
        thread.start();
    }

    /**
     * This is called immediately after any structural changes (format or
     * size) have been made to the surface.  You should at this point update
     * the imagery in the surface.  This method is always called at least
     * once, after {@link #surfaceCreated}.
     *
     * @param holder The SurfaceHolder whose surface has changed.
     * @param format The new PixelFormat of the surface.
     * @param width  The new width of the surface.
     * @param height The new height of the surface.
     */
    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height)
    {

    }

    /**
     * This is called immediately before a surface is being destroyed. After
     * returning from this call, you should no longer try to access this
     * surface.  If you have a rendering thread that directly accesses
     * the surface, you must ensure that thread is no longer touching the
     * Surface before returning from this function.
     *
     * @param holder The SurfaceHolder whose surface is being destroyed.
     */
    @Override
    public void surfaceDestroyed(SurfaceHolder holder)
    {
        boolean retry = true;
        while(retry)
        {
            try
            {
                thread.setRunning(false);
                thread.join();
            }
            catch (Exception e)
            {
                e.printStackTrace();
                Log.e(TITLE, "Error: ", e);
            }
            retry = false;
        }
    }

    /**
     * Implement this method to handle touch screen motion events.
     * <p>
     * If this method is used to detect click actions, it is recommended that
     * the actions be performed by implementing and calling
     * {@link #performClick()}. This will ensure consistent system behavior,
     * including:
     * <ul>
     * <li>obeying click sound preferences
     * <li>dispatching OnClickListener calls
     * <li>handling {@link AccessibilityNodeInfo#ACTION_CLICK ACTION_CLICK} when
     * accessibility features are enabled
     * </ul>
     *
     * @param event The motion event.
     * @return True if the event was handled, false otherwise.
     */
    @Override
    public boolean onTouchEvent(MotionEvent event)
    {
        return super.onTouchEvent(event);
    }

    @Override
    public void draw(Canvas canvas) {}
}

An der MainThread Klasse habe ich jetzt also nichts weiter geändert. Ausser eben im Contructor den GameView als zweiten Paramter eingetragen. [Code siehe Ausgangspost].

Wenn ich jetzt also ein neues Spiel mache - Airhockey zum Beispiel - dann sieht das ganze so aus:

Spoiler: Airhockey

public class Airhockey extends GameView
{
    ////////////////////////////////
    // Constances
    ////////////////////////////////
    private static final String TITLE = "Airhockey";

    ////////////////////////////////
    // Fields
    ////////////////////////////////


    ////////////////////////////////
    // Constructor
    ////////////////////////////////

    public Airhockey(Context context)
    {
        super(context);
    }

    ////////////////////////////////
    // Methodes
    ////////////////////////////////

    /**
     * All the update stuff goes here.
     */
    @Override
    public void update()
    {

    }

    /**
     * Initialization goes here.
     */
    private void init()
    {

    }

    ////////////////////////////////
    // Overriden Methodes
    ////////////////////////////////

    /**
     * This is called immediately after the surface is first created.
     * Implementations of this should start up whatever rendering code
     * they desire.  Note that only one thread can ever draw into
     * a {@link Surface}, so you should not draw into the Surface here
     * if your normal rendering will be in another thread.
     *
     * @param holder The SurfaceHolder whose surface is being created.
     */
    @Override
    public void surfaceCreated(SurfaceHolder holder)
    {
        super.surfaceCreated(holder);
    }

    /**
     * This is called immediately after any structural changes (format or
     * size) have been made to the surface.  You should at this point update
     * the imagery in the surface.  This method is always called at least
     * once, after {@link #surfaceCreated}.
     *
     * @param holder The SurfaceHolder whose surface has changed.
     * @param format The new PixelFormat of the surface.
     * @param width  The new width of the surface.
     * @param height The new height of the surface.
     */
    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height)
    {
        super.surfaceChanged(holder, format, width, height);
    }

    /**
     * This is called immediately before a surface is being destroyed. After
     * returning from this call, you should no longer try to access this
     * surface.  If you have a rendering thread that directly accesses
     * the surface, you must ensure that thread is no longer touching the
     * Surface before returning from this function.
     *
     * @param holder The SurfaceHolder whose surface is being destroyed.
     */
    @Override
    public void surfaceDestroyed(SurfaceHolder holder)
    {
        super.surfaceDestroyed(holder);
    }

    /**
     * Implement this method to handle touch screen motion events.
     * <p>
     * If this method is used to detect click actions, it is recommended that
     * the actions be performed by implementing and calling
     * {@link #performClick()}. This will ensure consistent system behavior,
     * including:
     * <ul>
     * <li>obeying click sound preferences
     * <li>dispatching OnClickListener calls
     * <li>handling {@link AccessibilityNodeInfo#ACTION_CLICK ACTION_CLICK} when
     * accessibility features are enabled
     * </ul>
     *
     * @param event The motion event.
     * @return True if the event was handled, false otherwise.
     */
    @Override
    public boolean onTouchEvent(MotionEvent event)
    {

        switch(event.getAction())
        {
            case MotionEvent.ACTION_DOWN:

                break;

            case MotionEvent.ACTION_UP:

                break;

            case MotionEvent.ACTION_MOVE:

                break;
        }

        return true;
    }

    @Override
    public void draw(Canvas canvas)
    {
        Paint paint = new Paint();
        paint.setTextSize(20);
        paint.setColor(Color.WHITE);
        canvas.drawText("bla",200, 200, paint);
    }

Ich glaube, dass das eine ganz solide Lösung sein sollte.
Sollte das jemand anders oder genau so sehen, würde ich mich über Feedback freuen.

PS: @Joose - warum genau sollte die Klasse nicht von Thread erben sondern implements runnable? Macht das einen unterschied?

 

[Joose]

Ich sehe es nur ein bisschen kritisch das die View sich ihren Thread selbst erstellt (trennung von logik und ui)
Aber kann dir da jetzt auch zu keiner wirklichen Lösung raten.

Ansonsten ist mir nur eine Kleinigkeit aufgefallen

    private MainThread thread;
.......
    private void init()
    {
        thread = new MainThread(getHolder(), this);
    }
......
    @Override
    public void surfaceCreated(SurfaceHolder holder)
    {
        thread = new MainThread(getHolder(), this);

        thread.setRunning(true);
        thread.start();
    }

In "init" und "surfaceCreated" erstellst du jeweils ein neues MainThread Objekt. Ich nehme an das in init wird sinnloserweise erstellt?
In "surfaceCreated" übergibst du als Parameter einen Holder, aber an den Konstruktor von MainThread übergibst du den Rückgabewert von "getHolder". Wozu dann der Parameter?

PS: @Joose - warum genau sollte die Klasse nicht von Thread erben sondern implements runnable? Macht das einen unterschied?

Man erbt von einer Klasse um deren Funktionalität zu erweitern. Das tust du aber nicht, du sagst dem Thread nur was er tun soll (implementieren der run Methode).
Außerdem kann man immer nur von einer Klasse erben sprich du bist hier eingeschränkter, Runnable hingegen ist ein Interface.
Ansonsten einfach mal hier ein bisschen lesen: http://javarevisited.blogspot.co.at/2012/01/difference-thread-vs-runnable-interface.html

PS: Ähnliches gilt auch bei JFrame, viele erben davon obwohl sie nur das Layout festlegen und Komponenten hinzufügen wollen. Das hat aber nichts mit der Funktionalität des JFrames zu tun

 

[Robat]

Jupp das Object in der init ist quatsch. Das ist noch von einem Versuch. Muss ich noch löschen.

Zum Thema Thread:
Klingt logisch, was du da sagst. Hab ich gar nicht drüber nachgedacht. Werde das aber aufjedenfall noch ändern auch wenn es in dem Beispiel kein großen Unterschied machen sollte. Aber da ich ja von Anfang an "gut und schön" programmieren will, werde ich das noch ändern.
Danke dir für deine Hilfe.
Immer wieder schön zu sehen, dass man sich von woanders Hilfe holen kann.

 

[Joose]

Eben, das mit dem Thread war ja als Anmerkung/Anregung gedacht und ist kein wirkliches Problem/Fehler im Code