Can this bit of code be more efficient?

Hello all.

(code screenshot attached at bottom)

I have 4 lines of code, each contains a FOR EACH regarding the same family - can it be made more efficient /faster, or would it be OK as is? There is also an odd bug or something in there too.

What it does, is it creates a target on each enemy on screen, when a button is held down, and kills all the targets when the button is released.

If the object in the family has a low strength variable less than 25, it changes the colour by changing the animation frame of the target.

The problem is, and what makes the bug odd, it works about 95% of the time.

Some of the family objects do not get a target set to them at all, although some of the very SAME objects in the family are not ignored. And sometimes the colour setting is wrong.

If I release the button which turns the CTRL_LOCK global to 0 and then hold the button down again which returns CTRL_LOCK global back to 1, the ignored family members are triggered correctly.

This also off / on procedure also fixes the family members that have an incorrect target animation frame.

To summarise my questions.

1. Are these 4 lines of code efficient in their structure? is there a way of using less FOR EACH loops?

2. Why would some family members get ignored or wrongly set by this code?

image of code snippet

https://imgur.com/a/YIrXQ

Humble thanks as always.

You are misusing "Trigger once" condition, and this is what probably causing those problems..

Your event 912 makes little sense, because you are checking a system variable, picking enemies instances and looping through them, and have "Trigger once" as the last condition, all in one event. It's impossible to tell how this will work.

Here is how I would change it:

Ctrl_Lock=1
Trigger once
    Enemies Y>10
    Enemies locable=1
    Enemies locked=0
    For each Enemies 
[/code:8m3olk1i]

Notice that I moved all picking conditions (Y>10, lockable=1, locked=0) before the For each loop. This reduces the number of instances that will be processed by the For each loop.

Also I suggest using sub-events, they make the code better structured and easier to understand.

In event 913 you don't really need the For each loop, you can remove it.

Event 914:
[code:8m3olk1i]
Ctrl_Lock=1
    Enemies locked=0
    Enemies strength<25
    For each Enemies 
       lockTarget UIDLink=Enemies.UID -> lockTarget  set frame to 1
[/code:8m3olk1i]

Event 915:
[code:8m3olk1i]Enemies locked=1
Enemies destroyed=1
For each Enemies 
       lockTarget UIDLink=Enemies.UID -> lockTarget  destroyed
[/code:8m3olk1i]

dop2000

Thanks for your help as always.. (check yr paypal!)

Is this how you pictured it?

https://imgur.com/a/KI2mlCV

As I am not 100% sure if thats what you meant, currently it's not working as well any more.

It is no longer checking for new enemies entering the screen when Ctrl_Lock=1

it's also displaying enemies that have strength<25 as the wrong colour now.

the original version constantly checked for new enemies, and changes in their strength, so if they were shot

a few times when locked, the target would update reflecting their current strength.

I PM'd you a private youtube video of how it was (almost) working if that helps. I'm at a point now where the amount of graphics and content is making the file a bit unsharable.

in your suggestion for event 914, it has Ctrl_Lock=1 check, when mine had Ctrl_Lock=0, I guess that was an error?

my apologies if I got something wrong in the edited code shot. I'm a designer / artist and my logic skills are a bit weak on the programming side. (can't be a master of everything!)

And thanks again.

I replied you in PM.