godotengine / godot-demo-projects

Demonstration and Template Projects
https://godotengine.org
MIT License
5.65k stars 1.58k forks source link

fix: mic_record script a bit for mac #949

Closed swarnimarun closed 5 hours ago

swarnimarun commented 11 months ago

Realised this demo was a bit broken while testing something irrelevant to the changes.

Also using all 3 names as iirc, OSX was what we used in 3.x. Though TBH not sure been a while since I used Godot 3. And I am not sure about the capitalisation part.

AThousandShips commented 11 months ago

What is the error you're seeing? Please provide some issue report for the reason behind this change

swarnimarun commented 11 months ago

Aah, sorry, somehow thought it was obvious. So if you don't use file:// with shell open on mac you will get -50 perm error, attached an image example below.

image
AThousandShips commented 11 months ago

That sounds like a bug to me, please open a report here

Also is this restricted only to macOS?

swarnimarun commented 11 months ago

@AThousandShips yep this is only specific to MacOS where the resource URI is required for correct call.

And there is already an issue for it, https://github.com/godotengine/godot/issues/52828, there was one even older than this(can't find that atm), I haven't been involved in Godot engine dev for a couple years so not sure why it hasn't been fixed but it should not be a big change, I could document a fix for engine there but I think the existing comments already make the issue quite clear.

AThousandShips commented 11 months ago

Okay! I asked for an issue before and got no answer so that's why I was confused 🙃

However this should probably not be solved here but as suggested in the issue by fixing it locally

I'd say the behaviour of OS should be unform, you shouldn't have to make specific changes like these

Calinou commented 6 months ago

@bruvzg Can we make it so we don't need to manually prefix file:// on macOS by modifying the engine's OS::shell_open() implementation?

This would prevent the need for this check in the project so the method can work in a cross-platform manner.

bruvzg commented 6 months ago

Can we make it so we don't need to manually prefix file:// on macOS by modifying the engine's OS::shell_open() implementation?

This would prevent the need for this check in the project so the method can work in a cross-platform manner.

What's the behavior on other platforms? It file:// is not required on Linux/Windows, I guess we can assume it's a file and auto add it if there's no URI schema in the shell_open argument.

bruvzg commented 6 months ago

Can we make it so we don't need to manually prefix file:// on macOS by modifying the engine's OS::shell_open() implementation?

https://github.com/godotengine/godot/pull/88126

aaronfranke commented 5 hours ago

Judging from the discussion, it seems that this PR is no longer necessary as of https://github.com/godotengine/godot/pull/88126. Closing. And thanks @swarnimarun for identifying the bug and leading us to a bugfix!