@Tef: It's great that you wrote that script, we're happy to see people writing and sharing useful procedures/functions. However there are some issues with it. It's bad practice to rely on something like:
for UnitsCounter := 0 to 999 do begin
It's true that unit/house IDs are assigned sequentially, but it's quite likely that there will be more than 999 in big games, since IDs are never reused so every new unit or house that is created takes up another ID. Sure you could increase it to 9999 or 99999 but that will start to become noticeable performance wise.
We realise that there is currently no correct way to gain access to all of the players units/houses/groups, which is something that you may want to do in certain scripts such as yours. Because of this we are planned to add functions which give you an array of IDs of every unit which is alive. It could have a parameter for player (-1 for any player) and another which is a set of what unit types you are interested in (which is good because we need to introduce people about the beauty of sets in Pascal somehow
). We'd also make versions of these for houses/groups. So that part of your script could change to something like this:
- Code:
var AllTheUnits: array of Integer;
...
MilitaryUnits := States.GetUnits(ACTIVE_PLAYER, [14..27]);
for I := 0 to Length(MilitaryUnits)-1 do begin
if (States.UnitPositionX(MilitaryUnits[I]) >= left) AND
...
Note that this is just the plan and it might change, but we certainly want to provide an alternative to iterating from 0 to 999 and hoping the game doesn't last long enough to exceed 999 units/houses created.
I also have some other feedback which might help you:
You can replace "(MilitaryUnitHasEnteredZone = false)" with "not MilitaryUnitHasEnteredZone". If you're more comfortable with the first version there's nothing wrong with that, I just wanted to let you know that both options exist.
Constants: Use "const" instead of "var", like this:
- Code:
function MilitaryUnitZoneTrigger(left: Integer; top: Integer; right: Integer; bottom: Integer): Boolean;
const
// define local constants
ACTIVE_PLAYER = 0;
LOW_UNIT_NR = 14;
HIGH_UNIT_NR = 27;
// define local variables
var
MilitaryUnitsAlive, MilitaryUnitsCounted, UnitsCounter, MilitaryType: Integer;
begin
There's also another way you could implement your script Run States.UnitAt() on every tile within the defined area, using for loops like this:
- Code:
for X:=Left to Right do
for Y:=Top to Bottom do
begin
U := States.UnitAt(X,Y);
...
end;
This will work quite well for small areas, that's how I did the king of the hill style maps. States.UnitAt is actually quite efficient because we have a reverse lookup to tell us which unit is standing on a certain tile (so it doesn't need to check whether every single unit in the game to see if it's standing there) On large areas the performance will be pretty bad because the number of tiles to be checked goes up exponentially (e.g. on a 100 by 100 area there's 100,000 tiles to be checked, not good).
I hope this helps, and thanks for posting your script here to help others
Right now we haven't seen any suggestions that really need classes... but as people's ideas get more and more complicated maybe we will.
