Can't Figure Out Why This Code Is Trying To Modify Closed Orders

Would you please explain your EA's objective so that I may offer a possible solution.

Does your EA have multiple open orders?

Does your EA trade multiple open orders on multiple currency pairs at once?

etc.

Thank you.

Its a martingale based system so its trying to modify the TP's of a basket of orders when a new basket trade is triggered.

 Yes it has 30+ open orders, about 5-6 each on different currency pairs and all using unique magic numbers. 

I suppose implicating this code into your EA could resolve possible problems when it selects it's orders-

OrderSelect(cnt_s3, SELECT_BY_POS, MODE_TRADES);
if(OrdersTotal()>0){

Also, you might possibly consider selecting each open order's profit individually. Once you selected all open order's profit, add all open order's profit. Once you add all open order's profit, write a block of code to check if a "profit target" has been reached by all open order's profit. Once "profit target" is reached, close all trades.

Thank you.

Should I add it to this

Or put it in its own if above or below that one.

If it was my EA, I would designate each open order a separate and individual block of code and yes I would apply the code to each and every open order block separately.

If I start "grouping" code, it can become difficult to isolate errors (buggies). Before I write code, I think of the best possible way to organize my code. After I organize it all in my

head, I type it out, block by block, compile it, test it, then repeat.

Thank you.

1. 
   For large amounts of code, attach it

   ---

2. for (cnt_s3 = OrdersTotal() - 1; cnt_s3 >= 0; cnt_s3--) {
   OrderSelect(cnt_s3, SELECT_BY_POS, MODE_TRADES);
   

   If the orderSelect fails, everything below is bogus. What are Function return values ? How do I use them ? - MQL4 forum

   ---

3. Not checking for magic number and pair, means your EA is incompatible with every other, including itself and manual trading.

   bool    MySelect(int iWhat, int eSelect, int ePool=MODE_TRADES){
       if (!OrderSelect(iWhat, eSelect, ePool)    ) return (false);
       if (OrderMagicNumber() == Magic.Number     ) return (false);
       if (OrderSymbol()      != analyze.pair     ) return (false);
       if (ePool != MODE_HISTORY                  ) return (true);
       return(OrderType() <= OP_SELL); // Avoid cr/bal forum.mql4.com/32363#325360
                                       // https://forum.mql4.com/30708
                                       // Never select canceled orders.
   }
   :
       for(int iPos = OrdersTotal()-1; iPos >= 0; iPos--)
       if (MySelect(iPos, SELECT_BY_POS)){
       :

   ---

4. OrderSelect(cnt_s3, SELECT_BY_POS, MODE_TRADES);
   if (OrderSymbol() != Symbol() || OrderMagicNumber() != MagicNumber_3 || OrderCloseTime()!=0) continue;

   When selecting by position you do not have to check the OrderCloseTime as it will always be zero with MODE_TRADES and non-zero with history. When selecting by ticket number, mode is ignored.

   ---

5. OrderModify(OrderTicket(), AveragePrice_s3, OrderStopLoss(), PriceTarget_s3, 0, Yellow);
   //===
   while(!OrderModify(OrderTicket(), AveragePrice_s3, OrderStopLoss(), PriceTarget_s3, 0, Yellow))

   Why are you modifying the order twice? If the first does not succeed, the second never will. If the first does succeed, the second never will (identical parameters.)

   ---

6. Why are you trying to modifying the price, instead of using OrderOpenPrice()
7. Why are you not testing OrderTakeProfit() vs PriceTarget_s3 to find out if you already did the modification? You would have found this had you done #2
   

   ---

8. If the order reaches the TP or SL, while the EA is in start(), you could get a closed ticket and thus get a modify failure. This is a race condition between the server and the EA. It should occur rarely, see #7. Log it and go on, and the next tick the loop won't find it.