-
Notifications
You must be signed in to change notification settings - Fork 286
Made a Geolocation extension #1943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as abuse.
This comment was marked as abuse.
Thanks! |
|
Here ! It's finally done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me! All protocols are being followed and I don't see any reason why this can't be merged now.
"use strict"; | ||
|
||
function getGeolocation( | ||
options = { enableHighAccuracy: true, timeout: 5000, maximumAge: 0 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with geolocation apis but will the timeout affect anything?
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really : timeout is the limit you give to geolocate the device.
In this case, the getGeolocation
function gives the browser 5000ms (or 5s) tops to geolocate the device.
It's the setTimeout
function that slows down/stops the code. See on mdn web docs.
extensions/SamuelLouf/Geolocation.js
Outdated
async getCurrent(args) { | ||
if (!(await this.isAllowed())) return ""; | ||
var coordinates = await getGeolocation(); | ||
if (coordinates.success == true) { | ||
return coordinates[args.WHAT]; | ||
} else { | ||
return ""; | ||
} | ||
} | ||
|
||
async isAllowed() { | ||
return await Scratch.canGeolocate(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yuri-kiss are we allowed to make block functions themselves asynchronous or should we put asynchronous functions inside them?
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find any blocks to actually get my location, which is a pretty necessary feature for a geolocation extension.
huh didn't notice that, maybe we're missing something? |
You are right. |
Dunno how I let that slip. |
Fixed it!! |
I'll check when I can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am quite impressed by this extension. All of the relevant blocks are there, and all checks have passed. @samuellouf You have my blessing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get longitude and latitude doesn't seem to work for me in Safari (the block says it's supported and it's not an API limitation), and in Firefox longitude is negative where for me it should be positive according to my Maps app.
Once these issues are fixed I think we'll be good to go
- Changed the default timeout - Fixed the `getCurrent` timeout (it wasn't taken into account until this fix)
I just made a few changes :
|
Perhaps there's something wrong with your computer or your phone. (No offense) |
@PPPDUD, could you check if the position the extension returns is valid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not the computer, I'm on a modern Mac with the latest software version. It works in Firefox, not Safari- it's not a browser limitation because the extension says I support geolocation and Apple's MapJS kit wouldn't work without geolocation capabilities, so something about the get location reporter doesn't work in Safari. Also, the longitude is still negative, where according to my maps app, it should be positive. |
That's interesting. I don't get why it isn't working. It should work. |
Should I add a "when user moves" block? (It would be a hat, ofc) |
Yes, but I'm not 180º on the other side of the world. A theory, I updated to the latest major macOS Tahoe release recently and it's a .0 version. I've discovered a few UI inconsistencies after the redesign so there may be potential functionality issues in the latest version, but it's very unlikely. |
Depends on whether users would use it and how performant it would be |
Hmm... it seems like I have location services disabled in settings and Firefox is attempting to override it, so it may be faulty. I'll test it later when I have time |
@samuellouf That is genius, please add that! You might want to add a way to enable or disable that block though, since it's probably gonna eat up time pretty easily (see @Brackets-Coder's comment). |
Added "onUserMove"
Done ! I added some code that sends the event, gets the threads to which the event was sent, and returns the value in the async getCurrent(args, util) {
if (!(await this.isAllowed())) return "";
var coordinates =
util.thread._coordinates || (await getGeolocation(this.options));
// read the coordinates in the thread if present OR get the current coordinates
if (coordinates.success == true) {
return coordinates[args.WHAT];
} else {
return "";
}
}
async changePositionWatching(args) {
...
this.watcherID = navigator.geolocation.watchPosition(
(pos) => {
var threads = Scratch.vm.runtime.startHats(
"samuelloufgeolocation_onUserMove"
); // Trigger the hat
for (var thread of threads) { // for each thread
var coords = pos.toJSON().coords; // Turn the coordinates into JSON
coords.success = true; // Add success
// @ts-ignore
thread._coordinates = coords; // Add the coordinates to the thread
}
},
() => {},
this.options
);
...
} |
Hello, folks!
I made a Geolocation extension.
Enjoy!