Farm Land Generator 1.5.3

[Moderator]

21:04, 16th Jun 2015
BPower:
Lot of things to discuss

 

[fireblasts]

There isn't alot out there like this useful

 

[Pinzu]

There isn't alot out there like this useful

Thanks, it's all I ever wished it to be!

 

[Almia]

Lacks documentation/comments

 

[Pinzu]

Lacks documentation/comments

Added documentation, didn't comment much but instead i cleaned the layout of the code, it should be a lot more clear for you, the moderator to read through it, as well as for the user to change it to suits his needs.

Coded it from scratch again. So I changed name and re-released it as "Farm Land Generator 1.0."

*edit
Update: Version 1.1

Change-log:
Added a flexible configuration for colors
Updated to the latest version of Pathing Library
Removed most local variables from Main
Changed Parameters in UpdateArea from real x, y to unit u
Fixed a bug where yield wouldn't display at 0%
Replaed SEARCH_RADIUS (constant) with a internal variable not to be touched by the user.
Added Setup Functions for TerrainYield, FarmGrid and FarmType. Not sure if they count as "leaks".

It's finished from my end. Feedback would be greatly appreciated.

Could add on Init to pick all preplaced farms and spawn farm land around it. But can't be arsed right now...

---

Is this still on somebodies (mod) to-do list? ^^

 

[Pinzu]

Update: Version 1.2
- Removed the Main function and separated the events to their own functions.
- Pre-placed farms now get farm land.


Last thing I'm gonna add is something that calculates the expected return upon building placement.

 

[Wasabee]

It was like the farms in Age of Empires.

 

[Pinzu]

It was like the farms in Age of Empires.

Ah there you could build farm land without farm buildings on them. This is more like BFME series. ;P

Anyway, having a worker work the land or the farms grow over time could be a interesting feature but I'm not sure if if its such a good idea given I want to keep it light for my map. However, removing the farm entirely and replacing it with "ground" only could be useful, but how would you kill it?

Anyway I have noticed a two issues in the current version:
1) Units can block farm land.
2) a lot of imported models are in that map for no good reason!

 

[Almia]

Is the code good bad, what should I change, can this be approved, anything?

Try contacting the moderator, IcemanBo

 

[Pinzu]

Version 1.3.
- Configuration moved to the top.
- Debugged PreplacedFarms
- Most junk moved inside a struct
- 3 API functions added.
- Added example usage.
- All variables were made privet
- Removed all imports that had gotten in the map by mistake.

 

[IcemanBo]

Hm, sounds interesting.
But isn't this more a system than a spell?

This used config code should not be in main code, as it's not relevant for system itself?
It's probably for demo?

Texttags don't need to be nulled. You can here read more about reference leaks: http://www.hiveworkshop.com/forums/trigger-gui-editor-tutorials-279/memory-leaks-263410/#post2661133

if (Expression == true)
->
if (Expression)

if (Expression == false)
->
if not(Expression)

When a farm gets destroyed, shouldn't be nearby farms potentialy be influenced in terms of getting new land for example?
Haven't tested the submissin yet but that comes in mind.

Haven't analazyed it yet, but idk if you cover all check cases correctly since a tile has multiple x/y values.

 

[Pinzu]

- Gonna look into the that. Does GetFilterUnit() cause leak?
- I also just realized updateArea could actually take in the radius, for example the GetWorldBounds() could be merged with it.
- Why should the configuration be outside if you want different types of farm areas or what? But you are right, it can be moved but then almost everything should be inside the struct (gx, gy and color), because the whole point of moving it would be to have more farm grid options and as its currently intended I don't see any big value in that.

When a farm gets destroyed, shouldn't be nearby farms potentialy be influenced in terms of getting new land for example?
Haven't tested the submissin yet but that comes in mind.

Haven't analazyed it yet, but idk if you cover all check cases correctly since a tile has multiple x/y values.

When a farm dies the farm tiles produced are first removed then this: call farm.updateArea(u) which updates the farmland of nearby farms. If a nearby farm can grow into that area it will.

Don't know exactly what you mean with the last part. But I've gotten it as correctly as I can, don't know what to do about the tile displacement effecting when a building is supposed to destroy farm land and not. You will see what I mean when you test it.

I want to point out that there is some redundancy in the algorithm as it checks all possible grids all the time, instead of those who are not already taken as well as not only checking the tiles which a building was placed on instead of the whole area. But i figured the effect is negligible.

Version 1.3.5
- shortend the function AddPreplacedFarms
- Changed .updateArea to include two options either search a radius or a rect.
- Changed some true false expressions.
- texttag no longer nulled. ;p

I think I want to add the following but I'm not sure
- Display estimated yield on construction (optional)
- Option to change a specifc farms yield to any percentage no matter the tiles its placed on.
- Cancel farms built on less then X yield automatically. (optional)
- Allow yields above 1.0 and below 0.0 and still keep the color-gradient.

 

[MiniMage]

I can't even start this thing. It gives me errors and dies.

 

[Pinzu]

I can't even start this thing. It gives me errors and dies.

Thats strange. Gonna take a look, downloading. Might be some configuration of jassnewgen pack? What happens when I open the map is that I have to save it again just before running it. Don't know if that is normal JNGP behavior or not, but I don't think its a problem. It's something I have to do with all my maps in the editor.

 

[MiniMage]

I tried that. Can't save the map. It gives me the same error.

 

[Pinzu]

I tried just throwing it in the scenario folder and playing it through wc3, the fact that it doesn't run bothers me, it says no memory was found: the same thing happens with my map project as well!? :/

However I can get it to run through the editor if I save it just before. Oh well, I'll begin debugging the trigger I suppose and then my map ;(, if anyone see any obvious flaws in the setup please tell me.

The memory error is so random, sometimes it happens but when I disable triggers under example usage it works, or that was just fluke luck, idk. Maybe it's because I didn't write those inside a scope.

Updated 1.3.5.c
- Changed the example usage triggers a bit, which I hope was causing it. I've managed to run it through wc3 a couple of times.
- Fixed a bug where dying farms would give land to farms under construction.

 

[BPower]

I think configuration should be done from a seperated scope/library ( piece of code ).
Just write the full API in the documentation, so users can look it up.
Add one line, explaining the purpose of the function/method.
Nice demo code in the attached map will help to quickly understand correct usage.

For AddPreplacedFarms I would run a single shot timer, which you then can destroy.
This way you don't have that unused trigger remaining for the whole game time ( cleaner approach )
Also I suggest you pass GetWorldBounds() directly as argument or use library WorldBounds as requirement.

farm_tile --> farmTile

Restructure your code. Methods beeing called should be placed above. Of course it compiles either way, but you would be suprised what the JassHelper does with your code.

if onFinish == true then --> if onFinish then
Why: if true == true vs true

I have more ideas, but at first I want to try out the demo map. ( tomorrow )

Just judging from the idea, it is a very cool system.

 

[Pinzu]

Updated to version 1.4, although I remade it from scratch again.

New features are:
- Calculating the expected yield of a unit before its completed.
- Moved the configuration and all triggers outside of the library, so the user can more easily add them where ever he wants instead of having duplicated triggers.
- There is now an event-variable to detect when a change happened as well as deltaYield variable to get how much was changed. Use it with any income variable you might have to change the income.
- Display was moved to the change-event trigger.

Also added most changes you suggested.

Will work some more on the documentation but it stands completed. If anyone can fix the mess I did with the hashtable it would be great, cus I "designed" it to work with multiple instances. But when I tested it, they appear to load the first configuration, couldn't be bothered fixing it as there is little use in it.

 

[WhiteFang]

This seems useful,i will check it out and see if it works for me

This is more like BFME series.

Those are fun games

 

[BPower]

The demo map is not so self-explaining to me.

1. function UnitAlive takes unit u returns boolean --> If you declare a function as UnitAlive, please make it private. I say this because there is a
native with the excact same name. native UnitAlive takes unit id returns boolean.
User may run into code conflict. The native is quite powerful, as it is very fast.
If you want to use it, you have to write it somewhere in your map.

2. call TimerStart(CreateTimer(), 0.1, false, function Farm.addPreplacedFarms) !! However is the timer is not required at all as units are preplaced before onInit is called.

3. library WorldBounds would be useful as requirement.

4. call DestroyTimer(GetExpiredTimer())

I'm writing from the phone, but there is more to do.
I continue next time.

 

[edo494]

move the functions below the Farms struct, you are generating countless unnecessary trigger evaluations right now

 

[Pinzu]

The demo map is not so self-explaining to me.

Suppose I could re-add the income trigger as a example usage or is there something else you are referring to perhaps? The problem is that I am not sure what someone else might need it to do, I only know what I need it to do. And I added more then that.

1. function UnitAlive takes unit u returns boolean --> If you declare a function as UnitAlive, please make it private. I say this because there is a
native with the excact same name. native UnitAlive takes unit id returns boolean.
User may run into code conflict. The native is quite powerful, as it is very fast.
If you want to use it, you have to write it somewhere in your map.

Okey, didn't know you could declare natives literally anywhere. In that case the user would see that it does exactly that and remove it? Anyway, changed it.

2. call TimerStart(CreateTimer(), 0.1, false, function Farm.addPreplacedFarms) !! However is the timer is not required at all as units are preplaced before onInit is called.

You are mistaken, i can't simply call the function addPreplacedFarms, wont work.

3. library WorldBounds would be useful as requirement.

Don't really get that library, but I'll look into it.

4. call DestroyTimer(GetExpiredTimer())

I don't have to null that timer?

move the functions below the Farms struct, you are generating countless unnecessary trigger evaluations right now

Okey, didn't know it worked like that.
___________________

Updated to version 1.4.2 Added all your suggestions except WorldBounds as I didn't find a good link/explanation and the timer changes, as I'm not sure if it will leak or not.

Other changes:
- New API:s, some BJ:s, for fun, but also just for the user to remove them when he is ready.
- Added a income trigger and a example of how to update the total income on eventYieldChange.
- Changed the calculateExpectedYield to take in x, y, and unitType instead of unit so that it will work for units before they are placed, for example on issued building orders or what have you.
- Some other things as well that I can't quite recall.

Noticed a few things I forgot to do: (Documentation error, wood income for test map, quite annoying without it, better in-game description on start) Will do that tomorrow most likely.

 

[edo494]

just because you call your functions BJ doesnt mean they are BJ, and the extensions to the names are purely useless and confusing, because now some users will think that these function exist even without your script since they are BJ(all BJ functions are defined inside blizzard.j, hence bj)

 

[Pinzu]

just because you call your functions BJ doesnt mean they are BJ, and the extensions to the names are purely useless and confusing, because now some users will think that these function exist even without your script since they are BJ(all BJ functions are defined inside blizzard.j, hence bj)

Yes, yes. Should I remove the functions entirely as they serve no real purpose or keep them?

 

[edo494]

I think dropping the BJ suffix will suffice. My resources also provide Functional interface, which does calls to the struct, like yours so I personally have no problem with that, just the BJ suffix

 

[Pinzu]

Updated! (WIP)
v. 1.5
- Rewrote it.
- Uses WorldBounds
- Now supprots multiple types of FarmLand (Testmap-contains 3 races)
- Support farm-tile variation, so you can have rough farm tile on low yield and lush farm tile on high yield.
- Now Uses Table
- Yield Display can now exceed 100%
- New functionality: Can change unit yield without any events and also reset it to the state it should be in.
- Can toggle a display variable on and off.
- Added Example Triggers to show how to use the custom yield change.
- improved income example delta in function OnYieldChange
- README

Known Issues
a) Not sure exactly how to use TableArray, so left a udg_hash inside the library. (minor issue)
b) When 100% yield is created you only get 97-99 income out of 100. (major issue)
c) A big enough group of units can actually block farm land creation. (minor issue)
d) Text-tag display is not working for more then 1 FarmLand instance. (major issue)

Any help on these issues would be greatly appreciated!

 

[edo494]

you are missing links for Native and WorldBounds(I dont even know what native means ), and for Table you linked to your user profile instead of the resource.

In description, you have refreshAreaAroundUnit, in code you have RefreshAreaAroundUnit(capital R at the beginning).

You mentioned native twice, but there is no native in your map script(I recommend adding the native UnitAlive takes unit u returns boolean at the beginning of the script.

FLAND_COLOR_MAX and FLAND_GRID_MAX should be included in the script itself, not in example script, because if I dont copy paste the example, then it wont compile again.

Again, you try to use FarmLand API before declaring the FarmLand struct, resulting in potential trigger evaulations(RefreshAreaAroundUnit).

I suggest adding the members of FarmLand as either readonly or private, I dont think users need to have access to the t1, t2, color etc naked(also possibly change it).

You should use table for both Color and Grid, because right now you can only have 819 colors and 273 Grid instances, so effectivelly 273 normally functioning FarmLand instances. With table, you are not limited on number of Grids nor Colors.

Your loop ends prematurely in CalcExpectedYieldXY, because you do exitwhen i == fl.grid.n before the body, but inside setupRadius you do exitwhen i == .grid.n after the body.

To be honest, I dont see why texttags shouldnt work(the function looks ok to me), and also I dont see the need for TableArray at all(well hashtable is hashtable, but ok)

 

[Pinzu]

- Added link to world bound, as for Native, it is already declared in the trigger "Native"? Didn't find any official link to native declarations either, think I picked it up from some help-question. I clearly felt that the users better visit my user profile instead.
- ok.
- ok.
- ok.
- How do you mean exactly?
- How many instances do people need? But i'll look into changing it.
- The issue was that the loop in radius was incorrect.
- Wanted to use table, because I never used it before. So I figured instead of messing with my map I'll do it here.

I found what was wrong with the text-tag, they have no color setup. fixed that. Maybe the colorizatin should be shared across all FarmLand instances?

 

[edo494]

basically, your function tries to access FarmLand API, but that needs to call methods inside FarmLand, and they are clearly declared below your function, so it will generate copy of the function, and potentially even trigger evaluation

 

[Pinzu]

basically, your function tries to access FarmLand API, but that needs to call methods inside FarmLand, and they are clearly declared below your function, so it will generate copy of the function, and potentially even trigger evaluation

Not that part, changed that. This: "I suggest adding the members of FarmLand as either readonly or private, I dont think users need to have access to the t1, t2, color etc naked(also possibly change it)."

Updated.

v. 1.5.1. and 1.5.2f
- fixed texttag bug
- fixed income bug
- fixed issue with the order of the function calls
- fixed documentation error
- removed customization of farm yield outside of event
- Table and Color struct now uses Table instead of variable array
- Units no longer block added farmland.
- Two new functions: AddFarm and RemoveFarm
- Made RefreshAreaAroundUnit privet.
- Bug fix in the test trigger.

Issues:
a) Not sure exactly how to use TableArray, so left a udg_hash inside the library
b) A big enough group of units can actually block farm land creation. (can be problematic)
c) Strangely enough version 1.5.1. has managed to break the update function around dead farms, no idea why as of yet.
d) Selection of units inside FarmLand area are deselected upon farm creations.

The function RefreshAreaAroundUnit uses the unit index of nearby potential farm blockers to store their temporary xy location. However, it might be a smarter move to utilize a table for this: what do you think?
________________________________________

I think it is finished now, sure i could add some more features but I finally think its at a good-enough quality. Thank you for your help and feedback. Hope you find this useful, albeit its a bit nished...

 

[Almia]

Ok so to help you, the current Hiveworkshop is nowhere to be found (because Nes removed all of his stuff here). In short, you are linking an empty resource in your library.

To solve the problem, here is the current link to WorldBounds:
https://github.com/nestharus/JASS/blob/master/jass/Systems/WorldBounds/script.j

 

[BPower]

    private function Init takes nothing returns nothing 
        local timer t = CreateTimer()
        call TimerStart(t, 0.1, false, function AddPreplacedFarms)
        set t = null
    endfunction

-->

    private function Init takes nothing returns nothing 
        call TimerStart(CreateTimer(), 0.1, false, function AddPreplacedFarms)
    endfunction

Also call DestroyTimer(t) --> call DestroyTimer(GetExpiredTimer())

Btw this can run onInit.

prove

function main takes nothing returns nothing
    call SetCameraBounds( -1536.0 + GetCameraMargin(CAMERA_MARGIN_LEFT), -2688.0 + GetCameraMargin(CAMERA_MARGIN_BOTTOM), 2432.0 - GetCameraMargin(CAMERA_MARGIN_RIGHT), 1536.0 - GetCameraMargin(CAMERA_MARGIN_TOP), -1536.0 + GetCameraMargin(CAMERA_MARGIN_LEFT), 1536.0 - GetCameraMargin(CAMERA_MARGIN_TOP), 2432.0 - GetCameraMargin(CAMERA_MARGIN_RIGHT), -2688.0 + GetCameraMargin(CAMERA_MARGIN_BOTTOM) )
    call SetDayNightModels( "Environment\\DNC\\DNCAshenvale\\DNCAshenvaleTerrain\\DNCAshenvaleTerrain.mdl", "Environment\\DNC\\DNCAshenvale\\DNCAshenvaleUnit\\DNCAshenvaleUnit.mdl" )
    call NewSoundEnvironment( "Default" )
    call SetAmbientDaySound( "AshenvaleDay" )
    call SetAmbientNightSound( "AshenvaleNight" )
    call SetMapMusic( "Music", true, 0 )
    call InitSounds(  )
    call CreateRegions(  )
    call CreateAllUnits(  )// Here all units are created
    call InitBlizzard(  )

//! initstructs
call ExecuteFunc("PathingLib___onInit")// Here is your library executed
call ExecuteFunc("FarmLand___Init")
call Test___Init()
call Display___Init()
call Income___Init()

//! initdatastructs
    call InitGlobals(  )
    call InitCustomTriggers(  )
    call RunInitializationTriggers(  )

endfunction

I would like to see that you variable naming follows the JPAG established by Bribe.
It's designed to stays a close as possible to the normal wc3 variable/function declaration.
---> fland_unit --> flandUnit

Why fland and not farmLand?

static group g in struct FarmLand should be privte. The two Table instances should
be at least readonly, if not also private.
Encapsulation is very important, so users do not accidently break your code.

unit enum ... yay ... no! My JassHelper highlights "enum",
if it does for you also then please change the variable name.

private integer i [icode=jass] should local integer in every respective function. [icode=jass]local texttag tt should be nulled at the end of function DisplayUnitYield
Texttag velocity, fadepoint, lifespan and size should be part of the global configuarion block and not hardcoded.

You always pass in null for enum in private function RefreshAreaAroundUnit takes FarmLand fl, unit u, unit enum returns nothing.
So basically this argument is useless ( private function ) and should not exist in the first place.

call DisplayUnitYield(farmland_hu, u, GetUnitYield(u)) always displays the same number for me. Is that correct?

Farm and Grid instances are never recycled, but farms can be destroyed!

Set to Need Fix

 

[edo494]

enum is not Jass keyword.

The complete list of Jass keywords:

type
extends
function
takes
returns
return
loop
local
if
else
elseif
endif
endloop
exitwhen
endfunction
globals
endglobals
debug
native
constant

Enum is not reserved(yet)

 

[BPower]

Thanks for enlightening me edo. My JassHelper highlights "enum" in gold like it does
for half of the JASS keywords (if, then, else, loop, ... ).
I don't excacly know who made the Syntax Highlighter for moyack's. I think it's from Van Damm.
Still it would be nice, if you change the variable name, because I think
this JNPG is used from many people.

 

[edo494]

well yea JNGP is used widely, but there are much worse cases, like good old private trigger trigger

 

[BPower]

or private timer timer

 

[Almia]

or private unit unit

 

[Pinzu]

- Improved the encapsulation, most stuff is put inside the struct. and all the struct variables are privet.
- Changed fland_someVariable into farmLandSomeVariable
- Changed the paremeters and enum variable in a couple of functions, mostly all functions were moved inside the struct again so farmLand fl is no longer required in the parameter.
- constant variables created for texttag setting.
- texttag now nulled
- timer t is no longer a local variabe.
- Added onDestroy methods to Grid, FarmLand and Color

There is not really a problem with running the thing on initialization, only problem is that the configuration must run before the function AddPreplacedFarms, so either I remove the timer trigger and let it actually be a callable function or leave it as it is. Another option is to remove that from the library as it is easy to recreate with the already available API, perhaps just move it to "FarmLand Main"

Have discovered one bug, once that is fixed I'll update it.

call DisplayUnitYield(farmland_hu, u, GetUnitYield(u)) Not sure how you tested that, but it should display the same number if nothing has changed and you entered "display".

 

[UmbraUnda]

Hi, I downloaded the map to test it out and it crashes everytime. Is it corrupt maybe? Please send me the mapfile using Pastebin or ask for my email.

I'd really like to use this for my upcoming castle builder.

Thanks!

 

[UmbraUnda]

bump