Don't notify on dispose to avoid exception

[percula]

I was encountering the following exception everytime a Confetti widget was disposed:

════════ Exception caught by foundation library ════════════════════════════════════════════════════
The following assertion was thrown while dispatching notifications for ConfettiController:
setState() or markNeedsBuild() called when widget tree was locked.

This _InheritedProviderScope<ConfettiController> widget cannot be marked as needing to build because the framework is locked.
The widget on which setState() or markNeedsBuild() was called was: _InheritedProviderScope<ConfettiController>
  value: Instance of 'ConfettiController'
  listening to value
When the exception was thrown, this was the stack: 
#0      Element.markNeedsBuild.<anonymous closure> (package:flutter/src/widgets/framework.dart:4297:9)
#1      Element.markNeedsBuild (package:flutter/src/widgets/framework.dart:4307:6)
#2      _InheritedProviderScopeElement.markNeedsNotifyDependents (package:provider/src/inherited_provider.dart:496:5)
#3      ChangeNotifier.notifyListeners (package:flutter/src/foundation/change_notifier.dart:226:25)
#4      ConfettiController.stop (package:confetti/src/confetti.dart:371:7)
...
The ConfettiController sending notification was: Instance of 'ConfettiController'
════════════════════════════════════════════════════════════════════════════════════════════════════

It was being caught by the framework, but started to clog up my logs. Simply avoiding a call to notifyListeners during dispose() fixes the issue.

[funwithflutter]

Thanks for the pull request. Do you mind giving an example of when this exception will be thrown. I tried recreating it by replacing the current route while an animation is playing. Dispose is called, and stop is called, but I'm not getting the exception.

[percula]

Yep, I replaced the example in the repo with one that throws the exception. It's on this branch: https://github.com/percula/flutter_confetti/tree/exception_example

Note that I added Provider. Perhaps that's the reason you didn't see it when you tried to replicate it.

[funwithflutter]

Ah okay. Because ConfettiController is a ChangeNotifier, and using it with Provider's changenotifier is causing the issue.

I like your suggestion with the pull request and I'll merge it in, but maybe in the future this needs to be refactored to not be a ChangeNotifier. A ValueNotifier might be better. Is there any reason you'd want ConfettiController to remain a ChangeNotifier?

[percula]

Since ValueNotifier is extended from ChangeNotifier, wouldn't we have the same problem?

[funwithflutter]

Since ValueNotifier is extended from ChangeNotifier, wouldn't we have the same problem?

Yes. But I'm thinking of making the ConfettiControllerState (an enum with fields playing and stopped) a ValueNotifier, not the ConfettiController class (this would then not extend anything). Because the ChangeNotifier is used just to update that "state" (ConfettiControllerState) within the ConfettiController.

The ConfettiWidget cares about that state and this is an easy change to make, but the problem is that some people may also care about that state change (if they want to know when the state changes, for example playing to stopped). So this might be breaking someones code if they listen to the ChangeNotifier. But it might be better to create a callback within the ConfettiWidget that will be called when the ConfettiControllerState changes, that people can use. Or they can also use the ValueNotifier.

But with this change it would mean you can provide the ConfettiController using a normal Provider (not ChangeNotifierProvider).

[shinriyo]

@percula Can you resolve?

[shinriyo]

@funwithflutter