fredsa / playn

Cross platform game library for N≥4 platforms
0 stars 1 forks source link

JavaNet.get()/post() executed asynchronously #102

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Purpose of code changes on this branch:

To make JavaNet get- and post methods behave like on other platforms - i.e. be 
executed asynchronously.

http://code.google.com/r/pareklund-playn/source/detail?r=24bc41f927dfc73953adb3d
1d0e987200f6521ed

When reviewing my code changes, please focus on:

* I employed a strategy where calls are executed in a new Thread each time. I 
felt it would be overkill to use a thread pool given that it is intended for 
single-user scenarios running on desktops. Please let me know if you think 
otherwise.

* I use a CallbackCaller class and a list (CallbackCallerList) of said 
instances to be able to call the callbacks (from JavaPlatform) in the game loop 
thread instead of executing the callbacks directly in the created threads. This 
in order to avoid any synchronization issues in game code.

* Inside the CallbackCallerList class I use a vanilla ArrayList for the 
CallbackCaller objects in conjunction with a separate lock object. The reason 
for not using Collections.synchronizedList() is because I want to reset the 
reference upon getting the callbacks to execute 
(CallbackCallerList.invokeCallbacks()), not just guarding parallel insertions.

It's my first review request in the PlayN project and the first time I use Git 
for anything else than checking out code so please be kind. ;) 

After the review, I'll merge this branch into:
/trunk

Original issue reported on code.google.com by par.ekl...@gmail.com on 3 Dec 2011 at 4:05

GoogleCodeExporter commented 9 years ago
Thanks for the contribution!

If you haven't already, you should fill out a CLA before this patch can be 
reviewed. We can then add your name to the contributors list:

http://code.google.com/p/playn/source/browse/CONTRIBUTORS

Original comment by mmast...@gmail.com on 8 Jan 2012 at 12:18

GoogleCodeExporter commented 9 years ago
Thanks for your reply. I have submitted a signed CLA now.

Original comment by par.ekl...@gmail.com on 8 Jan 2012 at 8:51

GoogleCodeExporter commented 9 years ago
I submitted my comments on your patch.

Note that it appears your email address isn't configured correctly in your 
.gitconfig (not sure if Google Code uses that email for notifications). You can 
fix them like so:

$ git config --global user.name "Firstname Lastname"
$ git config --global user.email "your_email@youremail.com"

Original comment by mmast...@gmail.com on 10 Jan 2012 at 5:33

GoogleCodeExporter commented 9 years ago
Thanks for your help and feedback.

I have now made changes as per your comments:

http://code.google.com/r/pareklund-playn/source/detail?r=c3ccb5beef0661f7fa49759
d58edd1202d0f0548

Next time I will make sure to fix basic things such as these before committing. 
I guess my eagerness to contribute got the better of me. ;-\

Original comment by par.ekl...@gmail.com on 11 Jan 2012 at 11:42

GoogleCodeExporter commented 9 years ago
I merged upstream changes into my local clone before committing the changes but 
it appears that I never managed to commit the upstream changes into my remote 
clone. That has now been amended. Sorry for that (as stated, I am quite new to 
git). Please let me know if you want me to do anything more related to this.

Original comment by par.ekl...@gmail.com on 12 Jan 2012 at 5:02