Hey Guest! Ever feel like entering a Game Jam, but the time limit is always too much pressure? We get it... You lead a hectic life and dedicating 3 whole days to make a game just doesn't work for you! So, why not enter the GMC SLOW JAM? Take your time! Kick back and make your game over 4 months! Interested? Then just click here!
I want to draw a string with 2, 3 or 4 different elements after each other. The elements are in a ds_grid called ds_AllAgents.
I now have this super inefficient bit of code and am wondering what are good ways to make it (way) more efficient.
Thanks in advance for your help.
At the very least, you want to move the check for instance_exists outside most of the for loops. You're running that check way too many times. You can actually move most of those checks out of the inner loops to reduce your runtime considerably.
As for making it efficient overall, I guess the question is what you're actually doing with all of this. There's probably a better way to organize your storage so you can loop through the array once rather than looping N^4 times.
Similarly, you check array_length() repeatedly despite not changing the size of the two arrays any. Personally, I would change these for loops to simpler repeat loops, but either way, storing the array lengths into local variables before the loops start should do well.
To a lesser extent, calling a local variable instead of the global variable holding your grid is faster and might read more cleanly: var _Agents = global.AllAgents, _Ag = global.Achtergrond; would suddenly make the below example chunk a lot quicker to read, write, and maintain without losing any meaning in your temporary names. Furthermore, you can combine all the separate if statements with && (which stops checking conditions once it hits the first false – called "short-circuiting operators"), and I plead that you use == for comparisons instead of = to prevent confusion and any future problems if GML finally drops this backward-compatible tolerance. (One last thing I can't leave alone; you use i once, then use i/2 repeatedly from thereon; I'd use a more meaningful name, myself, since I can see this column means something to you, but I'll just pretend I've set var i2 = i div 2; directly inside that first for loop.
(Does "Ag" make sense as a Dutch abbreviation? I abbreviate English "background" as "bg" in my code and elsewhere.)
Every bit of processing you can take out of the Draw Events and place elsewhere is for the better, unless things have drastically changed in my year+ absence. I try to do all this data collection and so forth in the End Step when possible, storing final results into variables (like your text to be drawn – all you really want in your Draw Event is to draw a small bit of text anyway, right?).
That said, whether you move the looping to End Step or keep in Draw, I see your draw_text functions are identical except for the actual text being drawn. In that case, it'd likely be better to save your text to a local variable, too, then draw it in a single function after all the if checks.
GML:
var _str_Ag = ""; // initialize before loops
_str_Ag = "Background: " + string(_Ag[l,1]) + ", " + string(_Ag[m,1]) + ", " + string(_Ag[n,1]) + ", " +string(_Ag[o,1]) + "."; // within your last if..then block
draw_text_ext(240,710,_str_Ag,15,360); // after your if..then blocks
But wow, really, from there, it appears all you're looping through this array for is to concatenate a bunch of text from the same column of the same grid based on simple conditions. Have you looked at the trinary operator ? : any? Truly, as Nidoking said...
As for making it efficient overall, I guess the question is what you're actually doing with all of this. There's probably a better way to organize your storage so you can loop through the array once rather than looping N^4 times.
P.S. Okay, okay, for my own personal fun and practice, I've decided to refactor the above code as Nidoking and I have suggested, but it still stands that there's probably a better way to do this once you explain a bit more.
Create
GML:
str_Achtergrond = "";
End Step
GML:
var _file = global.obj_agent_file,
_Ag = global.Achtergrond,
_Agents = global.AllAgents,
_len_file = array_length(_file),
_len_Ag = array_length(_Ag),
_str_Ag = "", _i = -1, _l = 0, _m = 0, _n = 0, _o = 0,
_is_4_l, _is_5_m, _6, _7; // this makes some checks below more efficient, but the checks look inefficient in the first place, plus magic numbers and meaningless variable names are icky
str_Achtergrond = ""; // I don't know if your setup really needs the text blanked/reset every step; just being safe
repeat (_len_file) {
++_i;
if (instance_exists(_file[_i * 2])) {
//% I still don't entirely understand the need for the loops within loops, but hopefully you'll shed light on this soon %//
repeat (_len_Ag) {
++_l;
repeat (_len_Ag) {
++_m;
repeat (_len_Ag) {
++_n;
repeat (_len_Ag) {
++_o;
//% quick prep work for all the hard-coded checks below %//
_is_4_l = (_Agents[# 4, _i] == _Ag[_l]);
_is_5_m = (_Agents[# 5, _i] == _Ag[_m]);
_6 = _Agents[# 6, _i];
_7 = _Agents[# 7, _i];
//% here's the way-too-hard-coded part that needs the serious reworking %//
if (_is_4_l && _is_5_m) {
_str_Ag = "Background: " + string(_Ag[_l, 1]) + ", " + string(_Ag[_m, 1]);
if (_6 == _Ag[0]) {
_str_Ag += ".";
} else if (_6 == _Ag[_n]) {
_str_Ag += ", " + string(_Ag[_n, 1]);
if (_7 == _Ag[0]) {
_str_Ag += ".";
} else if (_7 == _Ag[_o]) {
_str_Ag += ", " + string(_Ag[_o, 1]) + ".";
}
}
//% waaaiiit a minute, why manually check for array index 0 when you can start your m,n,o,etc. loops at 0?? I only start mine at 1 because you did... %//
str_Achtergrond = _str_Ag;
exit; // clearly, you only draw one thing, right? no need to keep looping, right? $:^ ]
}
} // o
} // n
} // m
} // l
} // if(exists)
} // i
