Skip to content
This repository has been archived by the owner on Nov 25, 2019. It is now read-only.

Multithreaded HTTPRequests #75

Closed
wants to merge 31 commits into from

Conversation

wilhelmberg
Copy link
Contributor

/cc @david-rhodes


Known issues:

… BergWerkGIS-multithread-filesource

# Conflicts:
#	src/Mono/FileSource.cs
#	src/Mono/HTTPRequest.cs
@wilhelmberg wilhelmberg requested a review from david-rhodes May 3, 2017 12:58
@david-rhodes
Copy link
Contributor

david-rhodes commented May 3, 2017

@BergWerkGIS I think we should implement different versions of IAsyncRequest. Developers should be able to choose which type to use (threaded, standard, editor, etc.). To support this, we may also need various implementations of IFileSource. The concrete type could then be constructed by a developer and injected into MapboxAccess?

Different implementations of IAsyncRequest may also help clean up the current version of #ifs. UWP could have one version, iOS another, etc.

Additionally, I’d like to remove any component/game object dependencies from the scene, if possible (sort of how we removed MapboxConvenience).

@wilhelmberg
Copy link
Contributor Author

@david-rhodes

I think we should implement different versions of IAsyncRequest. Developers should be able to choose which type to use (threaded, standard, editor, etc.).

👍

To support this, we may also need various implementations of IFileSource.

Not yet sure if this is needed. FileSource accepts IAsyncRequest, we might get away with different implementations of IAsyncRequest - depending on the needs that might arise and we are not yet aware of.

Different implementations of IAsyncRequest may also help clean up the current version of #ifs. UWP could have one version, iOS another, etc.

I suppose this will help a bit but will not get rid of the #ifs entirely:

We are including mapbox-sdk-cs as source files and not as DLLs.
All source is available all the time for all builds/platforms - at least I haven't found a way to link certain .cs source files to certain platforms, like we do for Mapbox.Json.dll.

Even if there will be a factory for initializing a concrete implementation of IAsncRequest depending on the currently active platform, eg Editor vs UWP, all source files will still be analyzed and the project will fail:
Running in Editor also includes the (currently fictional) UWP-HTTPRequest.cs and will fail as it doesn't know about UWP specify API.
Some #ifs will still be necessary.
How implementing platform specific code, factories, interfaces and #ifs will work together is still to be determined.

Additionally, I’d like to remove any component/game object dependencies from the scene, if possible (sort of how we removed MapboxConvenience).

Will revisit those changes to get a better idea of what that means exactly.

@david-rhodes
Copy link
Contributor

Not yet sure if this is needed. FileSource accepts IAsyncRequest, we might get away with different implementations of IAsyncRequest - depending on the needs that might arise and we are not yet aware of.

Sure, I was guessing what an implementation might look like. It seems more flexible that MapboxAccess should decorate an IFileSource. The request method might look something like:

        public IAsyncRequest Request(string url, Action<Response> callback) {
            return _fileSource.Request(url, callback);
        }

I suppose this will help a bit but will not get rid of the #ifs entirely:

At least the #ifs could be contained in just one place: the top of the file. Better yet, we may be able to use System.Conditional and have all that code stripped out depending on the platform being built.

@wilhelmberg
Copy link
Contributor Author

Superseded by #78

@wilhelmberg wilhelmberg closed this May 9, 2017
@wilhelmberg wilhelmberg deleted the BergWerkGIS-multithread-filesource branch May 9, 2017 13:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants