Followups on the performance discussion and improved pickup behaviour

[heckert]

There was some discussion on the foreach() within another foreach() and its impact on performance, compared to the caching optimization done in the Progression:

Another Foreach Foreach?

@sampattuzzi what are alternatives to this… this is the second foreach foreach you implemented after refactoring the stat one into a lookup table and telling us not to use a double foreach… Aren’t raycasts heavy to begin with? Now a double foreach on what we hit? Is this a good idea? …or am I just too concerned with performance… what if we were building this to mobile? Would this be laggy?

and

Two foreach loop unnecessary?

Hi guys, Could any one clarify my question below, to me, it looks unnecessary to do 2 foreach loops, at lease in this situation. I cann’t think of a scenario of an object has more than 1 scripts implementing IRaycastable interface. For instance, a hitObj can be either a combat, a pickup or a movement obj, once a cursor hover upon it, it will only show one cursor type because they all exclusive to another. I could not understand why we need an inner foreach loop to get all raycastComponents in …

One difference would be that in case we’re looping over the results of the RaycastAll() we just did, which on one side we expect to be a fairly small number of objects, and on the other side we do need to check all of them if there is at least one IRaycastable component present we need to deal with.
The most potential for optimizing this particular inner loop might be a rigid setup of the prefabs by only ever allowing one IRaycastable component per object, shortcutting the inner foreach() loop, and making sure the IRaycastable is near the top of the list of components to check.

In the case of Progression we’re doing foreach() loops on Dictionary objects, which might be more expensive than just a plain list (but then, one would hope the iterator would be fairly optimized to make it quick). And it’s not just the one instance of it where the player points somewhere, but an arbitrary number of objects querying Stats all the time.

Next to the buildup of the lookupTable we’ve already got, the most potential for optimization would be to refactor anything that pulls stat values out of the progression to a) cache it (if feasable), and b) refactor most of that into an event driven system, so any changes (like for example the player levelling up) will be broadcast and any object having a dependency on stats refreshing its data, as well as replacing most of the current Update() busy loops to be event driven as well. (No more polling on the UI data, for example!)

---

About the pickup behaviour…

Brian would you able to post your Collector example

Hi Brian, Would you be able to post your Collector example so that we still move over (or at least to) the objects we collect? I attempted based on your comment, but I had trouble implementing. Thanks.

I added the Collector to my project and had some weird issues where the player rotated on the x axis and other weird rotations happened (as well as my camera moving around). Eventually I found the culprit and a simple solution.

In the Collector we have this code:

        private void CollectBehaviour()
        {
            // Debug.Log("CollectBehaviour was called");
             transform.LookAt(target.transform);
        }

I determined it’s the LookAt() going wild, and actually it’s not necessary to do at all, since we would already be looking at the target from when we moved towards it. Therefore I just commented out everything inside CollectBehaviour().
(I did keep it in because one bit missing for it would be a proper “pickup” animation, as well as having a sound effect, and perhaps some VFX, or even a cinematic, for which this method would be the anchor point…)