Right now it's tagged as beta so we suggest you don't use it a a stable project ; feel free to play with it and give us your constructive feedback. If you need a stable version, please tell us and we'll try to help! By beta I mean: this is not stable, performance may not be great, but right now we'd like to discuss the interface, the needs of people who'd like to use it,

Loading the whole file into a string then calling strstr on it 3 times to get header, thumbnail and comments? Meh... it couldn't be worse...
Sorry if it sounds harsh but this kind of library can be use for batch processing so speed should really be kept in mind.

The_Big_Boo wrote:Loading the whole file into a string then calling strstr on it 3 times to get header, thumbnail and comments? Meh... it couldn't be worse...
Sorry if it sounds harsh but this kind of library can be use for batch processing so speed should really be kept in mind.

TGYoshi wrote:Well, that'll break terribly if the map's name contains "<header" or so...
Am disappointed, expected some official API and exposure of how the GBX format is structured.

I find your comments a bit harsh indeed. You need to take this as it is: a beta package that we're putting on github to see if people would be interested, to gather feedbacks, and to discuss features.

So yes, obviously, speed was not a concern when doing this PoC, and yes there may be some bugs, but it doesn't really matter, they are trivial to fix. Right now we're focusing on interface and architecture more than implementation details.

Try to look at the bigger picture: are people intersted in this kind of package? should we support it? what type of features do people want? etc. That's the kind of feedback we're interested in.

Please do not PM for support. Instead, create a thread so that everyone can contribute or benefit from the answer!

As you said in first post, there are already packages doing this and one of them is actually some code you own. This part of the Dedicated Manager was already a PoC which just needs some wrappings (mainly getters instead of public properties) and enchancements (like the gd part) but contains already more GBX structures and is easily extendable. So why doing it again from scratch?

Speed wasn't a concern, I already knew that. But that doesn't explain why the worst implementation was chosen. I mean, it's not as if you were PHP noobs, you shouldn't even think about this way to do it.

The_Big_Boo wrote:Speed wasn't a concern, I already knew that. But that doesn't explain why the worst implementation was chosen. I mean, it's not as if you were PHP noobs, you shouldn't even think about this way to do it.

We don't use this in production, and nobody does. If the package makes it to production, we'll fix the implementation. If it does not, we don't care.

The_Big_Boo wrote:Btw, talking about focusing on interface and regarding this[...]Classes named like "Dep", "Desc" or "Ident" and getters reflecting these

Not sure I understand your point. We chose, in the interface, to have a structure that mirors the Gbx, so that future evolutions of the format are compatible with evolutions of the interface. If you think it's not a good practice, i'm all ears.

gouxim wrote:Maybe in the end we should have left the implementation out to avoid discussing it instead of what's really at stake.

You're annoucing that you'll soon release this package, that people should start to play with it but now it's only about how it should look like? Then yes, we disagree about the meaning of beta, because it seems you're expecting pre-alpha feedbacks. If it's a beta, then I'm just pointing up issues. If you want to discuss ideas, make the thread in this way from the very beginning so people won't bother testing for nothing.

gouxim wrote:Not sure I understand your point. We chose, in the interface, to have a structure that mirors the Gbx, so that future evolutions of the format are compatible with evolutions of the interface. If you think it's not a good practice, i'm all ears.

Whatever the reasons these names have been chosen in the GBX format, it doesn't mean they should be kept when exposing them. Someone who doesn't know what the GBX names are will just wonder why he should call "getDeps" to get the dependencies or "getEnvir" to get the environment. It can make sense for those who know the actual GBX format (edit: imho it doesn't anyway), but others aren't even supposed to know there are XML parts (thus actual names) in the map file.

Whatever's in the dedicated-manager repository makes a lot more sense to me regarding the exposed api. getHeader(), getIdent()? People without knowledge about the gbx format are just going to get confused by that. $map->getName() is all people want. Even I don't know what the hell an Ident is in this context... getIdent isn't documented either.

One could argue that the GbxReader of the dedicated-manager should use getters instead of public properties, but besides that I don't see any reason for this either unless the implementation is proper.

Don't the "modern PHP standards" describe proper comment usage too? All I see is type declarations using comments because PHP's type system is stupid, but no proper description of what a method actually does?

Harsh or not, but simplicity and implementation both matter too. It doesn't seem to be your intention to create a full GBX parser by now, so why make it seem like one?