My Game Engine

Game Mechanics
#1

I’ve almost made my game engine working and had some example games. See it at http://game-engine-for-java.googlecode.com

Any improvements are appreciated.

#2

In MapView you are catching NullPointerExceptions. That is weird and bad style. If one of the parameters may be null, just check it before going on.
In Animation the synchronization is incomplete or the synchronization intention unclear.

#3

sry to say that, but your resource loading mechanics are very error prone. Using it is as handling a Eastern-Europe firecracker, it can explode anytime.

Thats because you use Threads with static fields without any locking.

#4

Nothing can go wrong with that code. The resources shouldn’t be accessed while the resource manager is loading them, hence the check for ResourceManager.isLoading() in the Javadoc before retrieving the loaded resources. There’s is nothing that can explode as long as you don’t add resources while the manager is loading (which you shouldn’t do) or try to access the resources before they’re finished (which is obvious). There’s nothing wrong with his code except that it allows you to do stuff that can be dangerous. To prevent all that you could add a boolean variable like this:


    public static final void loadResources(){
        numLoaded = 0;
        loading = true;
        new ResourceManager().start();
    }

You’ll also need to add [icode]loading = false;[/icode] at the end of the run() method too of course. You could also even eliminate the need for numLoaded.

Then add checks for this boolean in all unsynchronized methods like this:


if(loading){
    throw new IllegalStateException("Cannot do <that> while loading");
}

It would also be cleaner to change


    public static final boolean isLoading(){
        return !(numResources == numLoaded);
    }

to


public static final boolean isLoading(){
    return loading;
}

But really, if you just read the Javadoc and don’t use it in an intuitive way it will work 100%.

#5

Without synchronization, modifications of numResources and numLoaded from the resource thread might not be visible to client threads calling isLoading().

#6

They will be eventually. Synchronization is done automatically, it’s just not guaranteed exactly when it’s done. You can test it yourself: Make one thread increase an integer one step at a time until it gets to 10 or something, and have another thread do a busy loop checking the value of the integer.

#7

And that is the reason why you need to do it by yourself if you want a reliable solution.

#8

As 65K said not to leave catched exceptions, I did this only because my intension was to simplify the engine to ground as I thought that exceptions will be complex for beginners which is the statement “The simplest game engine” made as caption.

And theagentd I could not understand your example on synchronization. Could you please explain it? Also how did you insert inline code?

#9

I did not and would never give such an advice.
Of cource, exceptions should be catched and treated properly. But they should not be used as control flow construct or silently swallowed.
This was about NullPointerExceptions and I can’t think of any scenario where it makes sense to catch them instead of checking parameters instead or throwing an appropriate IllegalArgumentException for instance.

#10

Sorry for my english. Modified it now.

#11

Then change numLoaded to a volatile int then, but I’d love to see you prove how that changes anything at all. Plus in this case the point is kind of moot since the actual loading of the resources will be several magnitudes slower than any performance penalty introduced by synchronization.

#12

Can’t follow you anymore. Neither the original code nor your modifications have the required synchronization to assure visibility across multiple threads. Thus the class is not reliable to use. That is all I am saying.

#13

…

  • The variable numResources is set before the loading thread is started, so it is guaranteed to be correct for the resource loading thread.
  • If we make the variable numLoaded volatile, it will be synchronized every time it is read or written to, so the methods isLoading() and the updating of the variable in the loading thread will cause synchronization.

How is that not enough synchronization? You don’t even need any in the first place!

#14

You are saying to make numLoaded volatile but it worked without the volatile keyword every time I had tested. I’ll modify it though.

#15

The (out of order, x86) CPU is free to re-order these instructions as it sees fit, so there is no guarantee whatsoever that numResources is actually set prior to starting the thread. You really (!) need locking/synchronization.

Volatile does not cause synchronization, it disables caching. Incrementing a volatile variable from multiple threads will cause race conditions,

#16

Nope.

[quote]When a statement invokes Thread.start, every statement that has a happens-before relationship with that statement also has a happens-before relationship with every statement executed by the new thread. The effects of the code that led up to the creation of the new thread are visible to the new thread.
[/quote]
http://docs.oracle.com/javase/tutorial/essential/concurrency/memconsist.html

Okay, right. The variable is still just incremented from one thread, so that amount of “synchronization” is enough.

… which was exactly my point in the first place. It works fine as it is!

#17

Turns out I’ve been too cautious.

#18

Here’s a small test program which tests a busy loop waiting for a value to be changed by another thread. To avoid measuring time on two different cores (which I’ve heard can cause errors) I bounce the value back twice. The time printed is basically the time taken for one thread to signal another and then get a response back. This one uses a volatile int, but feel free to change that to a normal int and it works exactly the same.

[s]http://www.java-gaming.org/?action=pastebin&id=353

OLD:

Sample output:[/s]

The timings are between 0.073 to 0.075ms regardless of if you use volatile or normal variables.

NEW:
Turns out the test wasn’t that accurate. Here’s a new one which does more tests to get a more accurate reading.

http://www.java-gaming.org/?action=pastebin&id=354

[quote]This took 0.008ms, failed 0 checks before the value was changed.
This took 0.01ms, failed 0 checks before the value was changed.
This took 0.013ms, failed 1 checks before the value was changed.
This took 0.017ms, failed 0 checks before the value was changed.
This took 0.01ms, failed 1 checks before the value was changed.
This took 0.01ms, failed 120 checks before the value was changed.
This took 0.01ms, failed 0 checks before the value was changed.
This took 0.009ms, failed 0 checks before the value was changed.
This took 0.009ms, failed 1 checks before the value was changed.
This took 0.01ms, failed 0 checks before the value was changed.
This took 0.009ms, failed 0 checks before the value was changed.
This took 0.013ms, failed 0 checks before the value was changed.
This took 0.01ms, failed 1 checks before the value was changed.
This took 0.017ms, failed 0 checks before the value was changed.
This took 0.01ms, failed 0 checks before the value was changed.
[/quote]

#19

Hey guys, I’m confused. What changes do I need to make to the ResourceManager class?

Also how to insert inline code?

#20

Sorry about that, I kind of derailed the thread a bit. =S

Nothing HAS to be changed. It works perfectly fine right now. The only thing that could be improved is more error checking. Users of your engine that don’t know how your ResourceManager works internally might try to do things that aren’t legal. While the resource loading thread is running, you cannot modify the internal state from another thread unless you want undefined behaviour (= random crashes, half finished loading, etc). Therefore you could prevent the user from adding new resources and getting resources from the ResourceManager while it is loading by throwing an exception if it is currently loading. It’s just what I proposed in my post before: http://www.java-gaming.org/topics/my-game-engine/28232/msg/255918/view.html#msg255918. I can explain it in more detail if you want.