Java-Gaming.org    
Featured games (78)
games approved by the League of Dukes
Games in Showcase (426)
Games in Android Showcase (89)
games submitted by our members
Games in WIP (466)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1]
  ignore  |  Print  
  My Game Engine  (Read 2132 times)
0 Members and 1 Guest are viewing this topic.
Offline SHC
« Posted 2012-12-24 06:10:58 »

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.

Offline 65K
« Reply #1 - Posted 2012-12-24 08:42:22 »

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.

Offline Danny02
« Reply #2 - Posted 2012-12-24 13:54:03 »

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.
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline theagentd
« Reply #3 - Posted 2012-12-24 14:30:33 »

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.
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:

1  
2  
3  
4  
5  
    public static final void loadResources(){
        numLoaded = 0;
        loading = true;
        new ResourceManager().start();
    }


You'll also need to add
loading = false;
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:

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


It would also be cleaner to change

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


to

1  
2  
3  
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%.

Myomyomyo.
Offline 65K
« Reply #4 - Posted 2012-12-24 14:48:06 »

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

Offline theagentd
« Reply #5 - Posted 2012-12-24 15:09:01 »

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.

Myomyomyo.
Offline 65K
« Reply #6 - Posted 2012-12-24 15:37:14 »

They will be eventually. Synchronization is done automatically, it's just not guaranteed exactly when it's done.
And that is the reason why you need to do it by yourself if you want a reliable solution.

Offline SHC
« Reply #7 - Posted 2012-12-24 16:23:07 »

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?

Offline 65K
« Reply #8 - Posted 2012-12-24 16:34:50 »

As 65K said not to catch exceptions
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.

Offline SHC
« Reply #9 - Posted 2012-12-24 16:42:08 »

Sorry for my english. Modified it now.

Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline theagentd
« Reply #10 - Posted 2012-12-24 18:05:05 »

They will be eventually. Synchronization is done automatically, it's just not guaranteed exactly when it's done.
And that is the reason why you need to do it by yourself if you want a reliable solution.
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.

Myomyomyo.
Offline 65K
« Reply #11 - Posted 2012-12-24 18:18:48 »

They will be eventually. Synchronization is done automatically, it's just not guaranteed exactly when it's done.
And that is the reason why you need to do it by yourself if you want a reliable solution.
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.
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.

Offline theagentd
« Reply #12 - Posted 2012-12-24 18:25:16 »

...

 - 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!

Myomyomyo.
Offline SHC
« Reply #13 - Posted 2012-12-24 18:42:43 »

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

Offline Riven
Showcase Moderator

JGO Overlord


Medals: 612
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #14 - Posted 2012-12-24 18:59:58 »

- The variable numResources is set before the loading thread is started, so it is guaranteed to be correct for the resource loading thread.
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.


- If we make the variable numLoaded volatile, it will be synchronized every time it is read or written to
Volatile does not cause synchronization, it disables caching. Incrementing a volatile variable from multiple threads will cause race conditions,

Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings
Offline theagentd
« Reply #15 - Posted 2012-12-24 19:31:55 »

- The variable numResources is set before the loading thread is started, so it is guaranteed to be correct for the resource loading thread.
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.
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.
http://docs.oracle.com/javase/tutorial/essential/concurrency/memconsist.html

- If we make the variable numLoaded volatile, it will be synchronized every time it is read or written to
Volatile does not cause synchronization, it disables caching. Incrementing a volatile variable from multiple threads will cause race conditions,
Okay, right. The variable is still just incremented from one thread, so that amount of "synchronization" is enough.

You are saying to make numLoaded volatile but it worked without the volatile keyword every time I had tested.
... which was exactly my point in the first place. It works fine as it is!

Myomyomyo.
Offline Riven
Showcase Moderator

JGO Overlord


Medals: 612
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #16 - Posted 2012-12-24 19:37:43 »

Turns out I've been too cautious.

Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings
Offline theagentd
« Reply #17 - Posted 2012-12-24 19:46:41 »

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.

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

OLD:

Sample output:

Quote
Thread one started.
Thread two started.
Change detected by thread two, changed back.
This took 0.075ms, failed 0 checks before the value was changed.

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.

Myomyomyo.
Offline SHC
« Reply #18 - Posted 2012-12-25 06:06:25 »

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

Also how to insert inline code?

Offline theagentd
« Reply #19 - Posted 2012-12-25 16:39:13 »

Hey guys, I'm confused. What changes do I need to make to the ResourceManager class?
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.

Myomyomyo.
Pages: [1]
  ignore  |  Print  
 
 
You cannot reply to this message, because it is very, very old.

 

Add your game by posting it in the WIP section,
or publish it in Showcase.

The first screenshot will be displayed as a thumbnail.

xsi3rr4x (75 views)
2014-04-15 18:08:23

BurntPizza (68 views)
2014-04-15 03:46:01

UprightPath (80 views)
2014-04-14 17:39:50

UprightPath (65 views)
2014-04-14 17:35:47

Porlus (81 views)
2014-04-14 15:48:38

tom_mai78101 (105 views)
2014-04-10 04:04:31

BurntPizza (165 views)
2014-04-08 23:06:04

tom_mai78101 (261 views)
2014-04-05 13:34:39

trollwarrior1 (210 views)
2014-04-04 12:06:45

CJLetsGame (220 views)
2014-04-01 02:16:10
List of Learning Resources
by SHC
2014-04-18 03:17:39

List of Learning Resources
by Longarmx
2014-04-08 03:14:44

Good Examples
by matheus23
2014-04-05 13:51:37

Good Examples
by Grunnt
2014-04-03 15:48:46

Good Examples
by Grunnt
2014-04-03 15:48:37

Good Examples
by matheus23
2014-04-01 18:40:51

Good Examples
by matheus23
2014-04-01 18:40:34

Anonymous/Local/Inner class gotchas
by Roquen
2014-03-11 15:22:30
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!