Skip to content
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

Quiet some deprecated warnings. #277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaddTheSane
Copy link
Contributor

This will quiet some deprecated warnings in regards to Cocoa methods, including:

  • NSDocument hates NSString-based path methods: initial work on migrating to NSURL-based methods.
  • replace -[NSGraphicsContext graphicsPort] with -[NSGraphicsContext CGContext].

Note that I used tabs instead of spaces. I'll probably fix it in another commit.

@LIJI32
Copy link
Owner

LIJI32 commented Jul 31, 2020

Many of those are done intentionally:

  • NS_NOESCAPE and other "Swift Propaganda" such as nullability macros are intentionally not used. Swift will not be used in this codebase so there's no point I've read about NS_NOESCAPE and it seems to introduce optimization benefits to blocks in Objective-C (and C) as well, so that one's good.
  • self-> is not used by convention, -Wimplicit-retain-selfis intentionally not enabled in the Makefile
  • I intentionally use the deprecated NSString APIs instead of the NSURL APIs because they're easier to work with; they've been deprecated since 10.4 and Apple shows no intention of removing them
  • graphicsPort is not supported on Mavericks (10.9) which is still supported by SameBoy. I assume you're not using the Makefile for compiling? Properly selecting the right deployment target (10.9) should silence this warning.
  • bool is preferred for BOOL by convention, specifically so you don't have to add != 0 like that.

The other changes are welcome, once the tabs are fixed.

@MaddTheSane
Copy link
Contributor Author

Yeah, I used an Xcode project, mainly to see how it would compile under Apple Silicon.
One thing about BOOL under Apple Silicon: it'll by typedef'd to bool. Why Apple didn't do this in x86_64, or even i386, I don't know.

Ah, I thought -[NSGraphicsContext CGContext] was introduced in 10.9, but the header says it's only on 10.10.

My new commit removes the migration to NSURL, but still calls -fileSystemRepresentation.

@LIJI32
Copy link
Owner

LIJI32 commented Aug 1, 2020

The Makefile already compiles ARM-Intel fat binaries if you use CONF=fat_release

@MaddTheSane
Copy link
Contributor Author

Ah, so Universal compilation is already handled.
Another reason why I wanted to build it under Xcode was to use Xcode assets, including handling of dark mode. But you have code that works around it, so using Xcode assets would just be another thing to worry about for the makefile. Better to just leave it as-is for now.

Also, compiling Swift source using a makefile is a pain! I should know, my NetHack3D code has parts that are Swift, that have to interact with the build system.

@MaddTheSane MaddTheSane marked this pull request as ready for review August 1, 2020 15:24
@LIJI32 LIJI32 force-pushed the master branch 4 times, most recently from 4ebe973 to 0989ee2 Compare December 30, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants