libgdx / gdx-ai

Artificial Intelligence framework for games based on libGDX or not. Features: Steering Behaviors, Formation Motion, Pathfinding, Behavior Trees and Finite State Machines
Apache License 2.0
1.18k stars 241 forks source link

Replacing Array by Iterable where possible. #65

Closed NemesisMate closed 8 years ago

NemesisMate commented 8 years ago

When using the framework out of libgdx, using it collections can be a "problem" (the same collection isn't shareable between not-libgdx-collections-dependant code and dependant one).

In RadiusProximity and so, in ProximityBase the agent collection used is a 'com.badlogic.gdx.utils.Array' where it could be 'java.lang.Iterable'.

The only usage of the collection in those is to iterate it like:

for (int i = 0; i < agentCount; i++) {
    Steerable<T> currentAgent = agents.get(i);

I would suggest to use the List interface so you could continue using the ".get" method but Array doesn't implement it. However, I think in this cases is it perfectly possible the implicit iteration way:

for (Steerable<T> currentAgent : agents)

As I can see, the default iteration system on 'com.badlogic.gdx.utils.Array' reuses the same iteration object to be gc friendly.

davebaol commented 8 years ago

Yeah, I see your point and your solution should just work. I'm a bit reluctant though. You have to admit that using Iterable to declare a collection is very unusual.

NemesisMate commented 8 years ago

Yeep, I admit it and that's why I would suggest a "java.util.List" instead but libgdx collections are not meant to be friendly with others :S. Maybe libgdx developers could try to be more friendly on this aspect, I don't know.

I'm also reluctant with that solution but is the only one(that doesn't need much work) I could think :S and I just didn't wanted to post an enhancement without giving some ideas ;).

NemesisMate commented 8 years ago

However, if from a collection the only functionality used is it iteration I would say that the most flexible option is to use the Iterable interface. By other hand, it also would be more difficult to use any other functionality if needed on a future because of the refactor need but in this case it seems to be that this won't happen.

davebaol commented 8 years ago

Here you go. :smiley:

NemesisMate commented 8 years ago

Thanks ;).