Make macOS intro video handling more consistent with Windows.

[Hoikas]

Fixes two issues with the intro video handling on macOS:

• Mouse downs and mouse double clicks should cause the intro videos to terminate.
• On Windows, input events are always passed through to the engine, even if they cause an intro video to terminate.

[colincornaby]

Properties in Cocoa shouldn't have the underscore. This is syntactic sugar that will automatically create a member with a leading underscore, and then wrap it with the dynamic accessor/setter, so no need to put an underscore on manually.

[Hoikas]

I'm going to need some guidance on what to do because this ObjC stuff looks like a dumpster of punctuation exploded to me.

[colincornaby]

@property is syntactic sugar that does the following for @property plClientLoader* foo;:

• Defines a member _foo
• Defines a getter function foo
• Defines a setter function setFoo

So the leading underscore in the case of a property would be duplicative - and would create a _foo and set_Foo getter/setter function.

Obj-C does support normal members without the @property wrapper, but anything that needs to be exposed to other classes should use the wrapper.

[dpogue]

I've always had trouble wrapping my head around the @property stuff as well, but my understanding is as follows:

@property (modifiers) type* name will create a private instance variable accessible in the class called _name, and then create two public methods on class instances: name (a getter) and setName: (a setter). It will dynamically invoke those methods when accessing .name or assigning to .name, but they can also be invoke via the method names. You can provide your own methods with those names to override the default behaviour.

The part that always confused me was around the creation of private instance variables and when @synthesize is needed or not (IIRC it's not needed in current macOS versions unless you're trying to do something special).

As an aside, unless these properties are being accessed from multiple threads, we probably want the (nonatomic) modifier on them to avoid unnecessary locking around reads/writes.

[colincornaby]

As an aside, unless these properties are being accessed from multiple threads, we probably want the (nonatomic) modifier on them to avoid unnecessary locking around reads/writes.

This is a good call and a habit I need to get into.

The part that always confused me was around the creation of private instance variables and when @synthesize is needed or not (IIRC it's not needed in current macOS versions unless you're trying to do something special).

I've been calling this syntactic sugar but there are runtime features it needs.

In the 32 bit ABI those runtime features were missing (because Apple never breaks the ABI.) So developers had to define the member variable themselves, then define the property, and then use @synthesize to instruct the compiler to do the generation of the setter and getter.

In the 64 bit ABI the @synthesize is no longer needed. But if you ever target the 32 bit ABI again it may come up.

[colincornaby]

One other thing I thought of about properties: Accessing or mutating a property doesn't directly touch storage, it goes through the setters and getters.

For example:

• myObject.foo = bar is turned into [myObject setFoo:bar]
• myObject.foo is turned into [myObject foo]

The two styles of using the setters/getters are interchangeable and you might see both in the same code base.

If performance is a consideration you can always reference directly the member variable using _foo and bypass the getter and setter. But it can be considered bad manners especially if the setter has important things it needs to do when the value changes.

[dpogue]

If performance is a consideration you can always reference directly the member variable using _foo and bypass the getter and setter. But it can be considered bad manners especially if the setter has important things it needs to do when the value changes.

This is the other thing that I am always unclear on when doing stuff within class methods. Should I be using the _foo ivar access or always going through getters? It seems wasteful to go through getters, but also seems weird to have a mix of self.foo and _foo in the same method

[colincornaby]

If the property is atomic, self.foo will guarantee atomic access. _foo will bypass the wrapper and you won't get any of the side effects like atomic access. Which could be a bad thing.

Properties that declare themselves as copy will always produce a copied value. So there could be performance advantages to bypassing the wrapper. Same thing is true of adding memory management attributes to a property. Bypassing the wrapper might get you performance, but it will bypass any of the memory lifetime management that is supposed to be occurring.

The property also might be computed. I.E. there is no instance variable backing it and it's calculated on the fly.

Most Obj-C developers just use the property because they don't want to accidentally sidestep the property side effects and they don't want to cause problems if the property is changed.

[colincornaby]

This method should be removed after fixing the property name. The property is syntactic sugar that defines both this method and the member with the leading underscore.

[Hoikas]

This was copied and pasted almost verbatim from

Plasma/Sources/Plasma/Apps/plClient/Mac-Cocoa/PLSKeyboardEventMonitor.mm

Lines 75 to 78 in 5e46413

	- (plClientLoader&)gClient
	{
	return *_gClient;
	}

FWIW, the purpose of this function is to convert the plClientLoader& to a plClient*.

[colincornaby]

Ah. I think changing the property will push things around a bit here (the property is going to define a gClient getter method which will conflict with this.) I have to start work so I'll come back and look more at this later.

[colincornaby]

Sorry, thanks for waiting. For some reason I thought gClient was a plClient type and not a plClientLoader. This block of code will probably still get wiped out by the property change (in since the property will generate a gClient method and Obj-C doesn't support overloading.)

I'm re-reviewing the keyboard monitor code so some of this is me rethinking how that code was written.

Could we store the plClient reference directly instead? I'm not sure if there is a downside to doing so and it would clear up the type issues. I can make a similar change to the keyboard event monitor.

It's getting into Obj-C nuances - but reviewing the code (and considering @dpogue's performance questions) it looks like the keyboard monitor stores the value as a private member variable instead of a property. If we don't need the accessor and mutator that might be a slightly more performant way to declare the variable. But we're getting into weird Obj-C nuances that maybe aren't so important.

[Hoikas]

Unfortunately, the plClient* is not available immediately from plClientLauncher, so there are some race concerns there.

[colincornaby]

Ah, ok. That makes sense. The typical pattern here (in since again, no function overloading with the property name) would probably be to rename this something like gClientAsClient. Or something less weird.

You don't need to declare private functions in the header in Obj-C so you won't need to declare it there. You can write it as a function and continue calling it as a property (with the new name) in since properties are just a different way to call getter functions.