New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add GroupUtils and change Group.wurst to use its functionalities #25

Merged
merged 7 commits into from Jan 7, 2018

Conversation

Projects
None yet
2 participants
@IgorSamurovic
Copy link
Contributor

IgorSamurovic commented Jan 2, 2018

Also, .destr() now calls .release()
The code is tested in game, and it works with my systems (that use a bunch of groups) without leaking. I can't guarantee more than that. This is impossible to test because get handle id is unavailable during compiletime. This also breaks GroupTests.wurst because it now uses getGroup() during group iteration.

@Frotty
Copy link
Member

Frotty left a comment

some minor things.


function createGroups(int number)
let maxCreatePerCycle = 256
let actualLimit = IMinBJ(8191, GROUP_NUMBER_LIMIT)

This comment has been minimized.

@Frotty

Frotty Jan 3, 2018

Member

Use max and min from Maths package.

if compiletime
numTotal += createNow
for i = 1 to createNow
let g = CreateGroup()

This comment has been minimized.

@Frotty

Frotty Jan 3, 2018

Member

inline this

let createNow = IMinBJ(toCreate, maxCreatePerCycle)
toCreate -= createNow

if compiletime

This comment has been minimized.

@Frotty

Frotty Jan 3, 2018

Member

one should just implement ForForce for compiletime instead of this conditional imo.

Log.warn("Attemping to release an already released group!")
else
this.clear()
DestroyGroup(this)

This comment has been minimized.

@Frotty

Frotty Jan 3, 2018

Member

.destr()


public function group.hasNext() returns boolean
return FirstOfGroup(this) != null
iterUnit = FirstOfGroup(this)

This comment has been minimized.

@Frotty

Frotty Jan 3, 2018

Member

this will break if .next() isn't called instantly after hasNext or if someone only calls .next()

while itr1.hasNext()
    if itr2.hastNext()
        let u1 = itr1.next()
        let u2 = itr2.next()  // u1 and u2 will be equal

OR


someGroup.next() // ignore one unit
let second = someGroup.next() // this one will still be the first one

I know these use cases shouldn't happe and the user should call .next() only, but instantly after .hasNext() anyways, but yeah :/

/** Returns a reusable group from the GroupUtils stack is possible.
Creates new groups if none are found on the reusable stack.
If that is not possible, returns a non-recyclable group and displays a warning. */
public function getGroup() returns group

This comment has been minimized.

@Frotty

Frotty Jan 3, 2018

Member

move API functions above internal functions

This comment has been minimized.

@IgorSamurovic

IgorSamurovic Jan 4, 2018

Author Contributor

API moved to the top

@Frotty

Frotty approved these changes Jan 4, 2018

@IgorSamurovic IgorSamurovic deleted the IgorSamurovic:GroupUtils branch Jan 4, 2018

@IgorSamurovic IgorSamurovic restored the IgorSamurovic:GroupUtils branch Jan 4, 2018

@IgorSamurovic IgorSamurovic reopened this Jan 4, 2018

IgorSamurovic added some commits Jan 4, 2018

@Frotty Frotty merged commit f63ad25 into wurstscript:master Jan 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jan 7, 2018

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment