I’ve received an error during build testing that while pretty straightforward, i’m not sure why it’s occurring. It’s a simple null reference error, but appears to occur in UnityEngine.MonoBehaviour.CancelInvoke.
Looking over the code where it occurs (on super-rare occasion) the only thing i can think of causing it is if the method name passed to CancelInvoke wasn’t valid.
Log Error:
Snippet from Log
NullReferenceException: Object reference not set to an instance of an object.
at UnityEngine.MonoBehaviour.CancelInvoke (System.String methodName) [0x00000] in <00000000000000000000000000000000>:0
at MyProject.BaseComponent.OnGaze (System.Boolean isGazed) [0x00000] in <00000000000000000000000000000000>:0
at MyProject.XR.User+d__50.MoveNext () [0x00000] in <00000000000000000000000000000000>:0
at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00000] in <00000000000000000000000000000000>:0
at UnityEngine.MonoBehaviour.StartCoroutine (System.Collections.IEnumerator routine) [0x00000] in <00000000000000000000000000000000>:0
at MyProject.XR.User.ShowGazeCursor (System.Boolean isVisible) [0x00000] in <00000000000000000000000000000000>:0
at MyProject.SeqManager.InitUserAction (MyProject.StepAction stepAction, System.Boolean skipping) [0x00000] in <00000000000000000000000000000000>:0
at MyProject.SeqManager+d__27.MoveNext () [0x00000] in <00000000000000000000000000000000>:0
at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00000] in <00000000000000000000000000000000>:0
UnityEngine.MonoBehaviour:StartCoroutine(IEnumerator)
MyProject.XR.User:ShowGazeCursor(Boolean)
MyProject.SeqManager:InitUserAction(StepAction, Boolean)
MyProject.d__27:MoveNext()
UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr)
Method where the cancelInvoke is called
public void OnGaze(bool isGazed) {
if (actionType != Seq.Actions.Inspect) { return; }
if (isGazed) {
if (isCurrentlyGazed) { return; }
//looking at me
isCurrentlyGazed = true;
if (actionType == Seq.Actions.Inspect) {
Invoke("FinishedInspection", inspectionDuration);
}
if (highlight != null) {
if (SeqManager.Instance.IsCurrentInteractive(this)) {
highlight.color = Keys.Colors.GREEN;
} else {
highlight.color = Keys.Colors.RED;
}
}
} else {
if (!isCurrentlyGazed) { return; }
//looking at something else
isCurrentlyGazed = false;
if (actionType == Seq.Actions.Inspect) {
CancelInvoke("FinishedInspection"); //<- The spot
}
if (!isFinishedInspect) {
//Change Highlight Color, unless 'FinishedInspection'
if (highlight != null) {
highlight.color = Keys.Colors.GOLD;
}
} else {
//Change their Highlight Color
if (highlight != null) {
highlight.color = Keys.Colors.SKY;
}
}
}
}
protected void FinishedInspection() {
ToggleInteractive(false);
inspectionDuration = standardInspectionDuration;
if (onInteraction != null) {
onInteraction(this, actionType);
}
}
My first guess would be that this may be what happens if you try and cancel an Invoke that isn’t actually running. It’s been years since I’ve used Invoke so I don’t know for certain. If I had to take a stab at it I’d guess that this function runs once (or more) with isGazed=true and the Invoke is started, then a second time with isGazed=false and Invoke is cancelled, then a third time with isGazed=false and it’s trying to cancel something that’s already been cancelled.
There’s a reason I haven’t used Invoke in a long time, and that’s because honestly it’s just not very useful in comparison to its drawbacks. And especially if you have the possibility of canceling something, it’d be better to write that without Invoke. I can’t tell enough about what you’re trying to make it do to give specific advice, though. Maybe set a timeToFinishInspection = Time.time, and then in Update you can check against that variable? And to cancel it, just set it to -1.
I concur with @StarManta . Invoke and CancelInvoke have zero visibility into their operation, so there’s nothing to even reason about when they fail. Not only that because it accepts a string, it is subject to typing errors.
Instead, do that Star suggests and make yourself a timer, but do not check for equality: floating point numbers will almost never be equal when incrementally added to by fractional amounts.
Instead, load a countdown timer with your interval, and then reduce that by Time.deltaTime each frame, and when it passes to or below zero, do your action ONCE. This is commonly called a “cooldown timer.”
@StarManta Generally I’ve shied away from using unity’s Update method as they tend to get abused too easily. I think this entire project only has 2 or 3 of them total and their scope’s are tightly controlled as there can be several hundred to a few thousand components which inherit from this particular base class onscreen and awake at any given time waiting to respond to events.
@StarManta I’ll look into migrating away from the Invokes to a Timer class that’s more robust instead. I think something with event handlers and callbacks for timer Creation, Start, Stop, Pause, Expiry and Cancellation would to the trick. That way other systems can subscribe to not just the timers themselves, but also their states.
While there is certainly a wisdom in avoiding using Update more than is necessary, it does seem like you’ve prioritized things a little out of order if you’ve gotten rid of most of your usage of Update but you’re still using Invoke? Anything that is bad about overusing Update, Invoke has, as well as its own problems.
Good lord where does this nonsense come from?! Unbelievable this myth still persists in 2020.
Doing things costs time and resources. The name of the function being Update() has absolutely NOTHING to do with it. I wish people would stop it with this “hurr hurr Update bad…” stuff!
If you have a performance issue, attach the profiler and understand what is actually taking time and performance in your program. Anything else is indistinguishable from scrying or any other form of illogical thinking.
Depends on how many objects you have and how they’re organized. If you have thousands of objects with Update(), and you can switch to a manager class which calls them, that will save some overhead. (Of course at this point the advice is more likely “Switch to DOTS” rather than “write a manager class” for situations where this would be recommended.)
I would also recommend you to check coroutines. They can be very handy when you are trying to do stuff like the one you’re doing with Invoke. Also, they can be stopped and so on, to avoid having a persisten “if” check in your update.
Coroutines are really simple to use and become a life saver when you learn how to use them
The majority of the code base uses Coroutines to manage “animating” values. There’s only a few places where invokes are found.
It depends on the team, my last team i kept finding variable declarations and get components inside Update statement when they should have been cached, which added up fast, so i forced their disuse to reduce refactoring of code and am just now used to not using them. (Not update’s fault)
I understand, I’ve been moving the codebase away from Invokes when i have downtime between features
My higher priorities have mostly been addressed, specifically excessive GC allocations and un-cached raytrace lookups. (If the raycast result is the same as last frame, there’s no need to get the component again…)
