search

blitz-research / monkey

Blitz Research Monkey Source
226 stars 59 forks source link

HttpRequest for HTML5 target #18

Closed devolonter closed 11 years ago

devolonter commented 11 years ago

Thread in js will be never completed because BBThread class defines RunUNSAFE method as an abstract and overrides behavior by default.

I added the Run method, which is called by RunUNSAFE and completes the thread

blitz-research commented 11 years ago

I think it's correct the way it is.

Js (and as) don't have 'threads' as such, so RunUNSAFE actually executes on the main thread, ie: in js/as Thread.Start just calls RunUNSAFE directly instead of starting a new thread. The overriding implementation of RunUNSAFE is expected to start an asynchronous operation which, when complete, will set the thread.running flag to false - see asyncdataloader.js for an example. So threads in js/as are really just wrappers for async ops that finish when the async op finishes. Your fix would cause threads to finish once the async op finished starting(!), quite probably before the async op actually finished.

The implementation of RunUNSAFE in thread.js is not currently really used and may be confusing things a bit - this could maybe call a js only 'Finish()' method instead that sets running to false, which subclassing threads could call instead of setting thread.running to false directly.

devolonter commented 11 years ago

Oh, my bad, sorry.

I was just trying to implement httprequest for HTML5 and I was confused because the thread was never completed. Now I understand why it was done that way. Thank you for the answer and detailed info!

Now I'll try to implement it based on asyncdataloader. By the way, do you have any plans to implement something similar to httprequest for html5 and flash targets? As it's a little strange not to have it for a web-based platforms.

blitz-research commented 11 years ago

I do plan to get around to doing httprequest for html5, but I'm concerned it'll be less useful than people think as (I believe) xmlhttprequest can't be used 'cross site' without server cooperation, see: http://www.w3.org/TR/cors/

If I understand this correctly, this means you wont be able to access a REST-ful-ish server from html5 without the server's prior knowledge of the source site. But please correct me if I'm wrong.

XmlHttpRequest can still be used to access stuff on the server hosting the page, but then there's already LoadString, DataBuffer.Load. I guess they can't yet POST as-is, but how many people are going to be doing that?

devolonter commented 11 years ago

Yes, you're right. Cross-domain requests require special headers from server. Usually Access-Control-Allow-Origin: domain.name is enough. But in some cases you need a little more. So httprequest will be useless if you don’t have an access to the server.

In case you want to use cross-domain request without access to the server, you can use JSONP. Many public APIs are built on it. But in this case, we have the following limitations: we can only do GET requests, we can get data only in JSON.

POST requests are required less often, but I think it would be useful to have such opportunity.

In the current project I need to get data from another server in HTML5. So, in the next few days I’ll be playing with it. I'll let you know what came out of it, if you're interested.

skn3 commented 11 years ago

Definitely worth adding html5 support for httprequest. I already had one person pay me to write a html5 http post module, unfortunately it was before I knew there was an official httprequest module otherwise I would have extended that and pushed it to the official repo.

Sent from my iPhone

On 8 May 2013, at 23:06, Arthur Bikmullin notifications@github.com wrote:

Yes, you're right. Cross-domain requests require special headers from server. Usually Access-Control-Allow-Origin: domain.name is enough. But in some cases you need a little more. So httprequest will be useless if you don’t have an access to the server.

In case you want to use cross-domain request without access to the server, you can use JSONP. Many public APIs are built on it. But in this case, we have the following limitations: we can only do GET requests, we can get data only in JSON.

POST requests are required less often, but I think it would be useful to have such opportunity.

In the current project I need to get data from another server in HTML5. So, in the next few days I’ll be playing with it. I'll let you know what came out of it, if you're interested.

— Reply to this email directly or view it on GitHub.

blitz-research commented 11 years ago

Hmm...gonna think about this one for a while yet.

blitz-research commented 11 years ago

Hmm...closing the pull request closes the issue. Guess it doesn't hurt to have a bunch of pull requests open while we think about things...?

devolonter commented 11 years ago

Yes, unfortunately there is no way to close pull request without closing the issue. If I remove the branch with changes it will also close the issue. If changes are made in a separate branch that’s alright to keep pull request open.

devolonter commented 11 years ago

I did some work on it (see new commits). It works well for me in all major browsers. I hope this will help you to solve the issue.

blitz-research commented 11 years ago

Ok, will merge this eventually - just tried but it went a bit git-shaped. I think maybe I need to merge develop->master before trying to merge this, if this is based off of master?

blitz-research commented 11 years ago

Nope, no idea what's up there - I try and merge it, and there's conflicts in weird files like lang.cpp, win8game.cpp, transcc.monkey and a few other files totally unrelated to this fix.

Any idea why this might be happening? Could your original branch be very old or something? Perhaps just email me the files?

devolonter commented 11 years ago

Hmm.. It seems something went wrong when I synced my fork. I’ve sent you files on email. Now I will remove my fork, but before that I would like to know your opinion about that pull request - https://github.com/blitz-research/monkey/pull/17. What do you think about it?

blitz-research commented 11 years ago

Yes, I think the native debugger should be supported as much as possible so plan to add this.

On Thu, May 16, 2013 at 2:16 AM, Arthur Bikmullin notifications@github.comwrote:

Hmm.. It seems something went wrong when I synced my fork. I’ve sent you files on email. Now I will remove my fork, but before that I would like to know your opinion about that pull request - #17https://github.com/blitz-research/monkey/issues/17. What do you think about it?

— Reply to this email directly or view it on GitHubhttps://github.com/blitz-research/monkey/pull/18#issuecomment-17941025 .

devolonter commented 11 years ago

Nice! Thanks for the answer.