Question A more elegant null check?

I feel like these null checks could be a lot more elegant:

Code (csharp):

1.  
2. if((blockUp != null) && (blockUp.NeighborBlocks[Direction_Up] != null))
3. {
4.     if(blockUp.blockColor == blockUp.NeighborBlocks[Direction_Up].blockColor)
5.     {
6.         if((blockRight != null) && (blockUp.blockColor == blockRight.blockColor)) possibleMoves.Add(blockRight);
7.         if((blockDown != null) && (blockUp.blockColor == blockDown.blockColor)) possibleMoves.Add(blockDown);
8.         if((blockLeft != null) && (blockUp.blockColor == blockLeft.blockColor)) possibleMoves.Add(blockLeft);
9.     }
10. }
11.  

What do you guys think?

 

Is this trying to be a floodfill algorithm?

If so, stick it in a grid and do it with loops: loop across, loop down, and loop all four directions

 

Not exactly. It's part of the logic for searching for potential matches in my match 3 game.

 

Oh yeah, I'd definitely just make it a loop over your grid of pieces.

- clear your possible moves list

- iterate all possible 2-piece swaps and execute them one at a time, to a temp grid
--> (X,Y) -> (X+1,Y) swap
--> (X,Y) -> (X,Y+1) swap

That will be something like:

Code (csharp):

1. // looks for all swipe-right swaps
2. for (int y = 0; y < Down; y++)
3. {
4.   for (int x = 0; x < Across - 1; x++)
5.   {
6.     Block b1 = GetBlock( x,y);
7.     Block b2 = GetBlock( x + 1, y);
8.     if (b1 != null && b2 != null)
9.     {
10.        // swap the two above to make a new "trial" board
11.        // ransack the board for possible matches, adding them as you find them
12.        // see below To Find Matches
13.     }
14.   }
15. }

and then do the equivalent one to look for up/down swaps

To Find Matches

After you have made each test temp possible new grid,

- now iterate horiztonally from top to bottom, asking for 3- 4- or 5- sequences

- then iterate vertically from left to right, asking for 3- 4- or 5- sequences

If any of those have valid sequences, add the move you contemplated to your possible moves, go back and try the next swap combo.

 

!blockRight
!blockLeft

== null is not a requirement.

Elegance in which way?
Length of text

Or another way to know?


It's difficult to know what IFs you need to be keeping sovereign from eachother. And which of those you don't.

 

You are absolutely right!

The solution is: don't do null checks!

There's generally two ways to achieve this:

1. Have a valid object for all directions at all times. In cases where it used to be "null" it would simply have a blockColor of None - supposedly this is an enum, so the default (0) should be called None. If it's not an enum: make it one or use Color.magenta.
2. Convert all of these classes to a struct. They can never be null and their default state should have the None color as above (hence the enum because the default value 0 will map to None automatically).

 

You can write:

Code (csharp):

1.  
2. if((blockUp is not null) && (blockUp.NeighborBlocks[Direction_Up] is not null))
3.  

But I guess that's not your question

Since your blocks are nullable you can use something like that:

Code (csharp):

1.  
2.     if(blockUp?.blockColor == blockUp?.NeighborBlocks[Direction_Up]?.blockColor)
3.     {
4.         if(blockUp?.blockColor == blockRight?.blockColor) blockRight?.AddMove(possibleMoves);
5.         if(blockUp?.blockColor == blockDown?.blockColor) blockDown?.AddMove(possibleMoves);
6.         if(blockUp?.blockColor == blockLeft?.blockColor) blockLeft?.AddMove(possibleMoves);
7.     }
8.  
9.  

https://learn.microsoft.com/en-us/d...ss-operators#null-conditional-operators--and-

https://learn.microsoft.com/en-us/d...-reference/operators/null-coalescing-operator

 

But ONLY if they aren't UnityEngine.Object subclasses. The nullability ? check doesn't work (correctly) with Unity objects due to their schizophrenic nature of having both a C# and C++ object with lifetimes independent of each other.

 

Ouch that's harsh! Today we'd call it dissociative personality disorder.
Yes it's pathological but don't be an object-typist, that's so C++. /s

 

Problem is if you stop using objects chances are you'll stop using unity. If you can jump the hoops and hurdles. And can tolerate the MS documentations. And the Google results.
Or just go college.

 

Note that this only works with types which actually have a boolean conversion operator (which UnityEngine.Object has). Any other "normal" object does require an actual null check.

Also he wanted to check that the objects are NOT null. So using ! would be wrong. This would be correct (assuming we talk about UnityEngine.Object derived types):

Code (CSharp):

1. if (blockRight && blockUp.blockColor == blockRight.blockColor)

Regarding more clean code, when you have redundant code you usually want to try to put it in a loop. However if that's not possible it might help to combine the relevant code into a separate method that can be reused.

Code (CSharp):

1. void AddPossibleMove(Block basis, Block block)
2. {
3.     if (basis && block && block.blockColor == basis.blockColor)
4.         possibleMoves.Add(block);
5. }

So the 3 lines would become

Code (CSharp):

1. AddPossibleMove(blockUp, blockRight);
2. AddPossibleMove(blockUp, blockDown);
3. AddPossibleMove(blockUp, blockLeft);

Though keep in mind if you have this code in a tight loop that runs many times (million times) it's better to avoid method calls. Though the null check / boolean conversion of a UnityEngine.Object and the "Add" is already a method call, so the performance difference would be minimal.

 

Note that this isn't quite equivalent to the original if statements; the first if statement here would actually evaluate to true, even if blockUp is null.

In the end this would still end up working, because AddMove would only ever get executed for non-null blocks, in which case the preceding if statement would only evaluate to true if blockUp is also not null... but still, I think this is quite confusing to follow when written like this.

With a small adjustment I think the flow can be made a bit more intuitive:

Code (CSharp):

1. if(blockUp?.blockColor is Color requiredColor && blockUp.NeighborBlocks[Direction_Up]?.blockColor == requiredColor)
2. {
3.     if(blockRight?.blockColor == requiredColor) possibleMoves.Add(blockRight);
4.     if(blockDown?.blockColor == requiredColor) possibleMoves.Add(blockDown);
5.     if(blockLeft?.blockColor == requiredColor) possibleMoves.Add(blockLeft);
6. }

An extension method like this could perhaps also help a little bit with readability:

Code (CSharp):

1. public static class ListExtensions
2. {
3.     public static bool AddIf<T>(this List<T> list, T item, bool condition)
4.     {
5.         if(condition)
6.         {
7.             list.Add(item);
8.             return true;
9.         }
10.        
11.         return false;
12.     }
13. }

 

@SisusCo
Thumbs up especially for conditional methods, and these can also be made more proactive

Code (csharp):

1. public static bool AddIf<T>(this List<T> list, T item, Func<T, bool> condition) {
2.   var result = condition is not null && condition(item);
3.   if(result) list.Add(item);
4.   return result;
5. }

Edit:
(Now I haven't read the thread in its entirety, maybe I'm supplying the wrong argument here.)

 

@orionsyndrome I think you meant

item is not null

instead of

condition is not null

But yeah, that is a smart way to get rid of the need for manually null-checking each block before accessing their color.

One small down side to note is that it can result in some garbage being generated, due to a delegate being used. Though in some cases the compiler might be able to reuse a single cached instance.

(Again, not anything to worry about, unless used in a very hot path.)

 

Lemme see

desired:
condition null -> skip (& return false)
condition not null -> evaluate condition

Edit: This is intentional so that you can skip having to prepend the method with

Code (csharp):

1. #if UNITY_EDITOR || DEBUG
2. if(condition is null) throw new ArgumentNullException(nameof(condition));
3. #endif

Yadda yadda, for reasons I'll explain below

in effect:
condition null ->

condition is not null

(false), shortcircuit -> evaluates as false
condition not null ->

condition is not null

(true), calls delegate -> returns result

Nah it's ok.
You can evaluate the state of the item in the call site

Code (csharp):

1. myList.AddIf(item, x => x != null);

This is actually preferred since checking for null in a generic method is a hassle because without constraints you don't know whether it's nullable or not, then you have to use EqualityComparer<T>.default.Equals(...) which I find so ugly (on so many levels).

That's interesting. I thought lambdas would generate garbage only when capturing variables? A static lambda should have a general delegate declared by the compiler with the method itself declared as static (that's why I call it a static lambda, a lambda without external references does actually produce a static method). The reason for the garbage is the hot context allocated on the spot, used for the access by reference.

 

@orionsyndrome Ah okay then, I guessed incorrectly
I wouldn't necessarily have added a null check for the condition myself, which mislead me... But I guess it makes sense in the sense that it's a maybe-do kind of method, that returns a boolean to indicate whether or not anything was done. Errors/exceptions happening in those kind of methods can be surprising and undesired sometimes.

It's always a balancing act between trying to avoid sweeping potential user errors under the rug, and avoiding warning the user about something they did intentionally.

But then, is there any benefit to using a delegate instead of just a boolean here?

Code (CSharp):

1. myList.AddIf(item, x => x != null && x.blockColor != default);
2. vs
3. myList.AddIf(item, x != null && x.blockColor != default);

Sometimes this kind of pattern can be really powerful for chaining method calls (e.g. LINQ), but in this case I'm failing to think of any good use case.

Creating a new delegate instance always allocates memory.
For example this:

Code (CSharp):

1. Func<string> delegateInstance = greeter.GetText;

Gets lowered to something like this:

Code (CSharp):

1. Func<string> delegateInstance = new Func<string>(greeter.GetText);

And since delegate is a class type, the garbage collector needs to perform work for it get released from memory.

→ SharpLab

However, as you pointed out, with lambdas without any captured variables, and static methods, the compiler is smart enough to generate and reuse a single cached instance, so garbage isn't generated with each execution of the code.

So for example this:

Code (CSharp):

1. Func<string> delegateInstance = ()=>"Hello!";

Gets lowered to something like this:

Code (CSharp):

1. Func<string> delegateInstance = singleCachedInstance ?? (singleCachedInstance = new Func<string>(GetHelloString));

→ SharpLab

 

You can put the static keyword on a lambda to ensure that it is not a closure, not referencing external variables, in newer versions of C#. Then you know for sure that you are getting the garbage free compiler magic, and no programmer can accidentally convert the lambda from best case performance to worst case performance. But, I don't think it is supported yet by the .Net Standard 2.1 supported by Unity 2021+.

 

If you want to interrogate a model, there is! This is just a dumb example, normally you want to extract more meat than to just pass the item around. That's a good point in this particular example.

And obviously one can build whole clusters of these things.

Here's a better example (and there's more)

Code (csharp):

1. public Ledger<T> Where(ItemPredicate test, bool breakOnFail = false) {
2.   if(test is null) throw new ArgumentNullException(nameof(test));
3.  
4.   var result = new Ledger<T>();
5.   foreach(var item in _dict)
6.     if(test(item.Key, item.Value)) result.Add(item.Key, item.Value);
7.       else if(breakOnFail) break;
8.  
9.   return result;
10. }
11.  
12. public delegate bool ItemPredicate(T item, int count);

This is still context-specific, but elevates the predicate to the domain above, which is the place that owns this collection and so it knows better how to treat its items. (In this case the whole class is generic.)

My point was that it nudges the look & feel of the code into (sort of) "functional programming" which is one aspect of Linq that I really like, it's super expressive, generic and flexible, and provides a better separation of concerns.

You basically get a programmable worker function. Plus you get to hide the boring implementation details without actually compromising performance.

Exactly. But it was a good point you raised. It's good to know that capturing hides an insidious cost!

 

I remembered that this came up before…this assumes though you just have a separate data model for your board…

https://forum.unity.com/threads/how...thout-500-if-statements.1148084/#post-7405904

 

This got me to thinking... I've never made a Match3 game, despite arrogantly claiming that I know the problem space well enough to advise others.

Well, I went and made one yesterday... take a look! Full source and package included, including hinting system for possible moves you can make.

It's a fun problem space. Lots of logic to fiddle with and get right. I think I did? LMK if there's issues.

You can play it here in WebGL: https://kurtdekker.itch.io/match3-demo

Full package attached to this message.

 

@Kurt-Dekker
Great stuff! I have a Bejeweled Twist clone lying somewhere, but it was done as a dirty little secret minigame for an interactive movie player/chrome I did in Actionscript 3 quite a long time ago, and it's really not representative as a tutorial project, I hacked that thing in an afternoon. The whole game was around 200-250 lines of code (well that was the beauty of Actionscript) and now I'm sad because I've never made any kind of match-3 in Unity lol. I think I'll make one for the editor when I find some time

 

Astonishing.

Speechless.

Thank you!!!

 

While I've moved on from this null checking nonsense over the last few days thanks to the ?. operator, I must say I'm impressed with your Match3 game @Kurt-Dekker. That's some good stuff right there. Your next challenge is to make it never ending.

 

Just remember that Unity overloads the == operator, so for a UnityEngine.Object named Foo (Foo == null) may be true while the null conditional operator ?. treats the object as not null, if UnityEngine.Object.Destroy(Foo) has been called.

EDIT: I had called ?. the null coalescing operator, but ?. is a null conditional operator, ?? is the null coalescing operator.

 

This is a HUGE drawback! If you want to use the

?.

operator (which to me is needless streamlining, adding no value to the readability or maintainability of the code, two metrics that are very important to me), now every single time you Destroy() something, you MUST remember to set the variable to null.

And guess what? Sometimes you actually CANNOT do that, such as when something is private... OR...

For instance, if something is derived from UnityEngine.Object, AFAIK you could never trust

this?.Something();

to work. You would be calling .Something() on a just-destroyed

this

, as there is no way to set this to null.

Just code like normal whacko K&R C nerds like me:

Code (csharp):

1. if (this)
2. {
3.   this.Something();
4. }

Keep in mind things Destroy()ed are not Destroy()ed until end of frame

All the same goes for

??

If you don't believe me, try it right now! It's trivial to see how it fails instantly.

EDIT: In other news, remember that both

gameObject

and

transform

, if accessed within a Destroyed MonoBehaviour, will cause a null reference.

Therefore you must always check

this

because that is what you are actually saying:

transform

actually means

this.transform

Same goes for

gameObject

which actually means

this.gameObject

So if your

Something();

function fiddles with any UnityEngine-side stuff, it's gonna blow after your thingy is destroyed.

 

Thanks! I find that I really do learn best by doing.

Looking at the git log, I did about 30 minutes late at night just to kick off my ideas about matching and hinting, specifically to just DO what I had suggested in my reply.

That was a day or so after I first replied your post, and then I did 2.5 hours the following morning over my coffee and breakfast... that's my main daily morning gamejam time!!

I put in a final 1.5-hr burst in the evening to polish it for WebGL and itch and to make a mobile version to have handy on the phone.

Looks like a total of 38 commits... I definitely use the "creep and save" method when coding! I will often write an empty function with a bunch of "what needs to happen here" notes in it as comments and just commit it before I write any code... that has saved me so many times it's not even funny.

I know I did a few things in this project that I just deleted or abandoned or restarted... I feel that I learn so much just doing a feature that I want to abandon and restart, so often I do. git lets me abandon and yet not discard it:

- commit
- tag
- reset my branch back

now all the code has been snapped and held in place by the tag, and I can retry or amend the feature, revert or delete parts of it. I almost can't imagine working without git... it's that core to my coding experience.

Sometimes if I'm lazy and quick and want to abandon a particular experiment before committing, I just do a

git stash

and it all goes away into a stash, which effectively IS a commit for all intents and purposes, it's just out of sight.

But I digress. It is late. I hope you have lots of fun with your project! Come back and tell us more, it's always fun to see how people are getting on. Iterating logic like this is the CORE of so many game functions and subsystems.

 

What's it gonna take to extract a "best of Kurt" into a guide book for Unity users?

 

linking this from @Jasper-Flick here…

https://catlikecoding.com/unity/tutorials/prototypes/match-3/

 

While I love

?.

I also wholeheartedly agree with Kurt (who doesn't love it btw), because you have to be very mindful of fake null objects in Unity.

I personally use

?.

however only in the context of pure C#, and this is almost never the regular MonoBehaviour (aka scripting) context. In there I switch my brain to

==

and

!=

. I'm not happy about it, but it is what it is. I'd like to keep what little hair I have, thank you.

 

@Kurt-Dekker @Jasper-Flick Cool projects, but I bet you guys don't have AmogusMatch

 

@icauroboros I presume you meant Amongus? Amogus sounds like something Peter Molyneux promised but never delivered.

 

sus