- Home /
Call method based on an Audiosource's time
Hello I'm fairly new to Unity and programming in general what I'm trying to do is call methods at specific times on an audiotrack. What Ive done is have an array TimedEvents that stores values of times in a song I want something to happen and a list of bools Executed to make sure each thing gets called only once.
void Listen() {
if (CurrentOrder == TimedEvents.Length) {
return;
}
else if(CurrentOrder < TimedEvents.Length)
{
if (!Executed[CurrentOrder] && audiosorce.time >= TimedEvents[CurrentOrder])
{
FireGun(CurrentOrder);
print("Oh Hello There " + CurrentOrder);
CurrentOrder++;
}
else
print("Not Yet Fam");
}
}
void FireGun(int limit)
{
Gun.Shoot();
Executed[limit] = true;
}
Now this thing works fairly well for me but since I didn't get any actual education for programming other than self-studying on youtube. I have very little self-confidence in what I've made. So I'm asking if there a better way to do this. Because the way I'm doing it now doesn't feel like the correct way to go about things.
Answer by JVene · Nov 15, 2018 at 07:35 PM
Not bad for a new programmer.
I show in the following some observations intended to help you advance your skills, so don't take any of this as merely critical points for the sake of being critical, but for the sake of helping you see more in your code and how to structure things.
The first clause in List() is a quick exit when the rest will be unnecessary, and that's a good choice. However, the "else" clause is superfluous. If the return is executed, nothing else can execute anyway, so the else has no actual functional effect on your code.
If you change the first clause from '==' to '>=', then the only possible 'error' would be < 0, which if you know can't happen (since you likely initialize to zero and only increment that value, and/or make the integer unsigned) makes the second test unnecessary. If CurrentOrder is >= the limit, and you use unsigned or otherwise know it can't be < 0, you have fully tested CurrentOrder to be in range, so you can eliminate the test for < limit.
Think about what happens if by some unimaginable circumstance Executed[ CurrentOrder ] happens to be true. In the code as expressed that would end all processing of any values from CurrentOrder + 1 to the limit - 1. This is because CurrentOrder will never be incremented in such a case. It is food for thought, the solution may not be to increment CurrentOrder in such a case. Consider what CurrentOrder is and how you control it. If you ever execute FireGun, you increment CurrentOrder. Is there ever a chance one could happen on the present value of CurrentOrder where CurrentOrder is not incremented? I don't see any such thing here. That means you can interpret the increment of CurrentOrder AS THE BOOL you are using the Execute array for. Unless you have some reason to list entries from Execute (and I can't see why), you really don't even need the Execute array for anything. Its functional purpose is handled by the increment of CurrentOrder, as along as the only way CurrentOrder is ever incremented is when FireGun is called on it.
Now I come to the point where I must assume, based on how this code is handling time, that it polls for the passage of time. What happens if two or more TimedEvents have already expired? That is, if the poll speed for this code is slow enough that two events are so close together that you'll miss a scheduled moment, what happens? Well, it will get 'caught' on the next occasion you call Listen(). That seems well enough, but what if the next event(s) expire(s) by the time Listen is called again? This creates a kind of traffic jam. It may be that you never have data in TimedEvents that could even create this situation, and so I'm making a pedantic point you might consider irrelevant. However, from my viewpoint, where I can't see that data, I have to think more generally. For this algorithm to more accurate process entries from TimedEvents, where the rate at which Listen is called may be too low for the resolution of those times in TimedEvents, the test here should well be constructed as a loop, so that within this one call to Listen all entries that have expired are processed (or at least skipped, since it may not be desirable to fire the gun in such close succession). If that latter point is valid, perhaps a loop following this code, but still inside the Listen method, should loop through all expired entries (which would, due to the speed of the processor, be very close in time relative to firing a gun). This would mean that the CurrentOrder would 'naturally catch up' if it was late relative to the schedule.
There's a finesse you might consider, too. Listen apparently polls, and while it isn't a heavy thing, you should call Listen on the boundary of an Update, which limits the amount of processing time sufficiently to keep it from being a problem. However, under any other circumstances that do not limit the rate at Which listen is called, or for additional efficiency, you could implement a faster test based on how far in the future the next TimedEvents should fire. Listen is a method, which itself has some overhead just to make the function call. You already stop things from happening with early rejection in your first test, when CurrentOrder exhausts the schedule. However, if you knew the next event was one full second in the future, you could avoid even making the call to Listen while that time has not yet expired. I'll leave the details of how that might be implemented for you to ponder, because in part I don't think you need it here (but you may in future work). The point is that you can obviate any real work for the CPU in such cases that you can avoid the method call because you know there's quite a bit of time before the next event fires. In other examples in my own work history, I've used timers instead of polling the current time of some timer. The timers are handled by the operating system (or some object/system), such that a call is made after some given time has expired. In such a method, I take the time of the next event, subtract the current time to calculate how long to wait. I set a timer for that duration. When that event fires, it concludes by repeating that act for the next event. With such a construction there is no polling.
Thank you for the reply, and I apologize for the delay in answering. Your comments helped a lot and I've made several changes to Listen.
Your answer
Follow this Question
Related Questions
Multiple Cars not working 1 Answer
How do I check if a spot in a cell is missing? 2 Answers
Learn C# In General Or Learn It With Unity? 1 Answer
Movement using the Scrollbar 1 Answer
Trying to use lists to check for available spawn points 2 Answers