libgdx/gdx-ai

Make protected members of IndexedAStarPathFinder

Gadyel opened this issue · 1 comments

I want to extend IndexedAStarPathFinder to add some extra functionality but i can't do noting because access type of this members are default:

public class IndexedAStarPathFinder<N> implements PathFinder<N> {
	IndexedGraph<N> graph;
	NodeRecord<N>[] nodeRecords;
	BinaryHeap<NodeRecord<N>> openList;
	NodeRecord<N> current;

Exist any reason why these members can't be protected, or is better to be default?

This is a constant nagging problem with quite a bit of libGDX-related code. There is a performance penalty from changing to public (or, I think, protected) fields when private or package-private ones are currently used in critical sections of code. It isn't an especially large penalty, but it may be worse on Android and GWT, where devices are normally lower-end. The general consensus seems to be that since all of this code is open source, you should copy the class and make appropriate changes for your use case rather than inheriting, which seems like somewhat like a circa-1986 programming practice. That's actually unfair to the developers of titles like Starflight for the IBM PC, which actually had a more sophisticated internal setup than the inheritance-weak OOP we're relegated to here... and it was released 33 years ago. It appears the goal is to eke every possible cycle of performance out of the library, which seems oblivious to the fact that all developers using gdx-ai chose a managed language, not assembly, for good reasons and are limited to a several-year-old VM version for mobile compatibility.

I have copied code from libGDX probably more times than I can count, and it's quite often because of something broken that I need to work around but can't via inheritance. It looks like gdx-ai has been abandoned, since it's stuck on libGDX 1.9.8 when 1.9.10 has been out for a while now, and there seems to be one code commit in all of 2019. It seems like a fork of gdx-ai to bring it up-to-date may be a better option. It could also include one of the actually-rather-important PRs about IndexedAStarPathfinder's (mis?)usage of == for equality rather than .equals().