[3.2] [iOS] Fix compilation warnings and deprecated API #42459

naithar · 2020-10-01T11:02:27Z

Fixes #29800 for 3.2
Deprecation and errors in 4.0 has already been addressed with Vulkan implementation.

Most of the code changes duplicate the same changes made in 4.0. There is no major deletions of previous/deprecated functionality, which I plan to do in followup PRs to make 3.2 file/code structure more like 4.0 to make it easier for future iOS PRs to be cherry-picked.

I've also reenabled werror for iOS GitHub workflow.
Increased deployment target in iOS template to 10.0, since 3.2 Godot is built for 10.0 version anyway.

~~#41173 (which fixes #41058 for 3.2) could conflict with this PR, but shouldn't be anything major.~~

akien-mga · 2020-10-01T11:07:46Z

platform/iphone/app_delegate.mm

@@ -301,6 +304,7 @@ - (void)setControllerInputHandler:(GCController *)controller {
 				OSIPhone::get_singleton()->joy_axis(joy_id, JOY_ANALOG_R2, jx);
 			};
 		};
+		/*


Should this code be removed instead of commented out?

And what about the following #ifdef ADD_MICRO_GAMEPAD which references being disabled by default due to not having API 9? I guess it's an outdated comment, but does this code work?

Well, it's currently commented out at 4.0, so I've decided to keep it. Will delete it after rebase.
As for ADD_MICRO_GAMEPAD Godot is already built for SDK 10, so it should be safe to use.

Note that nothing defines ADD_MICRO_GAMEPAD currently, so this code might require testing to validate it before enabling it.

Yeah, I've noticed that. While compilation seems to be working, I can't really test micro (?) gamepad as I don't have one.

akien-mga · 2020-10-01T11:10:07Z

#41173 (which fixes #41058 for 3.2) could conflict with this PR, but shouldn't be anything major.

I just merged it, so a rebase will be needed.

bruvzg · 2020-10-01T11:16:31Z

platform/iphone/in_app_store.mm

@@ -189,11 +192,9 @@ - (void)paymentQueue:(SKPaymentQueue *)queue updatedTransactions:(NSArray *)tran
 				int sdk_version = 6;

 				if ([[[UIDevice currentDevice] systemVersion] floatValue] >= 7.0) {


Do we need ancient code for iOS 7?

We don't need it, but it's still exists in 4.0. And I planned to remove it later PRs to keep this PR as simple as possible. I'll remove it required. :)

Is it possible to build it using iOS 7 target? If it's not (with is definitely the case for 4.0) there's no reason to keep it.

Godot uses -miphoneos-version-min=10.0 so it's not. I'll change the code then

Yeah if you want to keep this PR a simple backport of master changes, and then make two new PRs for further deprecation fixes for master and 3.2, that's fine with me. The comments made here can be used as actionable feedback for this follow-up PR.

Yeah, I think that should work better. But I'll rebase and remove some commented parts, as they wouldn't affect anything anyway.

naithar · 2020-10-01T12:59:16Z

iOS getting ./core/math/octree_definition.inc:689:46: error: ambiguous expansion of macro 'ABS' [-Werror,-Wambiguous-macro,1] error.
For some reason I don't get it on my computer

akien-mga · 2020-10-01T13:08:40Z

There are also conflicts with MIN and MAX, all taken from /Applications/Xcode_11.7.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.7.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h

But I don't remember seeing those in the past either in build logs.

For ./core/math/octree_definition.inc, it was added recently via #41123. Maybe it needs to use Math::abs instead of ABS? But several other core classes use ABS so not sure why it's an issue with octree.

naithar · 2020-10-01T13:10:07Z

Managed to get this error after rebasing again. Still not sure what caused it.

bruvzg · 2020-10-01T13:12:32Z

I'm getting it without this PR too, so it's not related. Using following command (same as CI)
scons -j2 verbose=yes warnings=all werror=no platform=iphone target=release tools=no

All definitions in SDK have ifndef ABS (MAX/MIN), ~~it's probably issue with included order (Godot math header is included after system one and redefine it).~~ and Godot header also have ifndef ABS, that's strange.

naithar · 2020-10-01T13:19:53Z

Also got:

[  6%] Compiling ==> platform/iphone/os_iphone.mm
In file included from platform/iphone/os_iphone.mm:40:
In file included from ./servers/visual/rasterizer.h:35:
In file included from ./servers/visual_server.h:36:
In file included from ./core/math/geometry.h:34:
./core/math/delaunay.h:108:21: warning: ambiguous expansion of macro 'MAX' [-Wambiguous-macro,1]

Moving system headers at os_iphone.mm to top before #include "os_iphone.h" seems to be fixing this issue. But I'm not sure why this problem hadn't occurred before.

The other strange thing is that this issue hadn't occurred in master branch

naithar · 2020-10-01T13:24:39Z

platform/iphone/os_iphone.mm

+// System headers are at top
+// to workaround `ambiguous expansion` warning/error
+#import <UIKit/UIKit.h>
+#include <dlfcn.h>


Temporarily moved to top to check if it helps.

naithar · 2020-10-01T14:02:36Z

Seems like it can also be solved by -Wno-ambiguous-macro, but I'm not sure that it's better, then keeping system header at top.

Found this: https://sqlite.org/forum/forumpost/6d0e24a98e, not sure if it's related, but looks like it.

naithar · 2020-10-01T14:30:23Z

@akien-mga linux build failure seems to be unrelated to this PR:

ERROR: get_language_code: Invalid locale 'C'.
87
   At: core/translation.cpp:945.
88
ERROR: set_locale: Unsupported locale 'C', falling back to 'en'.
89
   At: core/translation.cpp:969.

Change deprecated method calls to new ones. Guard iOS version dependant functionality behind availability checks.

akien-mga · 2020-10-01T17:12:28Z

Thanks!

naithar requested a review from akien-mga as a code owner October 1, 2020 11:02

akien-mga added enhancement platform:ios topic:buildsystem topic:porting labels Oct 1, 2020

akien-mga added this to the 3.2 milestone Oct 1, 2020

akien-mga requested a review from bruvzg October 1, 2020 11:04

akien-mga reviewed Oct 1, 2020

View reviewed changes

bruvzg approved these changes Oct 1, 2020

View reviewed changes

naithar force-pushed the ios-warnings-3.2 branch from 236752d to d977b6f Compare October 1, 2020 12:41

naithar force-pushed the ios-warnings-3.2 branch from d977b6f to f7ab360 Compare October 1, 2020 13:23

naithar commented Oct 1, 2020

View reviewed changes

iOS: fix deprecations

3386fac

Change deprecated method calls to new ones. Guard iOS version dependant functionality behind availability checks.

naithar force-pushed the ios-warnings-3.2 branch from f7ab360 to 3386fac Compare October 1, 2020 15:10

akien-mga approved these changes Oct 1, 2020

View reviewed changes

akien-mga merged commit dbb1df2 into godotengine:3.2 Oct 1, 2020

naithar deleted the ios-warnings-3.2 branch October 1, 2020 17:14

This was referenced Oct 2, 2020

[4.0] [iOS] Switch to ARC. Refactoring and cleanup. #42503

Merged

[3.2] [iOS] Port 4.0 changes #42582

Merged

akien-mga mentioned this pull request Oct 5, 2020

iOS build issue with SDK 12.4 via osxcross #42584

Closed

naithar mentioned this pull request Oct 31, 2020

Export game as embeddable iOS view to easily use non-Godot code (Android already supported) godotengine/godot-proposals#1473

Open

This was referenced Jan 12, 2021

[BUG] Error compiling Godot for iOS with module for version 3.2.4 beta 5 poingstudios/godot-admob-android#41

Closed

[iOS] [3.2] iOS Plugins #41340

Merged

akien-mga modified the milestones: 3.2, 3.3 Apr 21, 2021

akien-mga mentioned this pull request Nov 10, 2021

[TRACKER] Compilation warnings raised on iOS builds #29800

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3.2] [iOS] Fix compilation warnings and deprecated API #42459

[3.2] [iOS] Fix compilation warnings and deprecated API #42459

naithar commented Oct 1, 2020 •

edited

Loading

akien-mga Oct 1, 2020

naithar Oct 1, 2020

akien-mga Oct 1, 2020

naithar Oct 1, 2020

akien-mga commented Oct 1, 2020

bruvzg Oct 1, 2020

naithar Oct 1, 2020

bruvzg Oct 1, 2020

naithar Oct 1, 2020

akien-mga Oct 1, 2020

naithar Oct 1, 2020 •

edited

Loading

naithar commented Oct 1, 2020

akien-mga commented Oct 1, 2020 •

edited

Loading

naithar commented Oct 1, 2020

bruvzg commented Oct 1, 2020 •

edited

Loading

naithar commented Oct 1, 2020 •

edited

Loading

naithar Oct 1, 2020

naithar commented Oct 1, 2020 •

edited

Loading

naithar commented Oct 1, 2020

akien-mga commented Oct 1, 2020

		@@ -189,11 +192,9 @@ - (void)paymentQueue:(SKPaymentQueue )queue updatedTransactions:(NSArray )tran
		int sdk_version = 6;

		if ([[[UIDevice currentDevice] systemVersion] floatValue] >= 7.0) {

[3.2] [iOS] Fix compilation warnings and deprecated API #42459

[3.2] [iOS] Fix compilation warnings and deprecated API #42459

Conversation

naithar commented Oct 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Oct 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naithar Oct 1, 2020 • edited Loading

Choose a reason for hiding this comment

naithar commented Oct 1, 2020

akien-mga commented Oct 1, 2020 • edited Loading

naithar commented Oct 1, 2020

bruvzg commented Oct 1, 2020 • edited Loading

naithar commented Oct 1, 2020 • edited Loading

Choose a reason for hiding this comment

naithar commented Oct 1, 2020 • edited Loading

naithar commented Oct 1, 2020

akien-mga commented Oct 1, 2020

naithar commented Oct 1, 2020 •

edited

Loading

naithar Oct 1, 2020 •

edited

Loading

akien-mga commented Oct 1, 2020 •

edited

Loading

bruvzg commented Oct 1, 2020 •

edited

Loading

naithar commented Oct 1, 2020 •

edited

Loading

naithar commented Oct 1, 2020 •

edited

Loading