Manialib\Gbx: read Gbx files with PHP

[gouxim]

Fellow coders,

We are working on a simple PHP package for reading metadata information from Gbx files.

Please note this is not a Windows program to read Gbx files (checkout GbxDump for this) but library to implement this in your PHP application.

There are other packages doing this (see GBX Data Fetcher or this part of the Dedicated Manager). Manialib\Gbx follows modern PHP best practices (namespaces, PSR's, tests, etc.).

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,

Now because everyone's love a little bit of code:

Code: Select all

use Manialib\Gbx\Map;

$map = Map::loadFile('/path/to/my/map.map.gbx');

// Get the map author
$author = $map->getHeader()->getIdent()->getAuthor();

// Save the map thumbnail
$map->getThumbnail()->saveJpg('/path/to/my/thumbnail.jpg');

Project page https://github.com/manialib/gbx

You can also discuss it in French: viewtopic.php?f=19&t=30425

[toffe]

[Rots]

Oh great Looks nice I'll give it a try on my manialink for sure

[The_Big_Boo]

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]

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.

[gouxim]

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.

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.

[The_Big_Boo]

Well, let me explain a bit more then.

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.

Btw, talking about focusing on interface and regarding this:

Manialib\Gbx follows modern PHP best practices (namespaces, PSR's, tests, etc.).

=> Classes named like "Dep", "Desc" or "Ident" and getters reflecting these
=> A single 90 lines long function doing the whole parsing

It's harsh again, but this package, beta or not, hasn't the usual quality of your other packages. The recent one for parsing strings was way better for instance.

[gouxim]

So why doing it again from scratch?

Experimenting with github, interfaces, phpunit, phpcs, methodology for defining architectural choices, packagist, workflow with our internal tools, etc. There are other internal concerns. Starting from scratch helps answering such questions if no constraints whatsoever.

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.

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.

A single 90 lines long function doing the whole parsing

Those lines are precisely about implementation, not interface, therefore I don't care for now.

It's harsh again, but this package, beta or not, hasn't the usual quality of your other packages. The recent one for parsing strings was way better for instance.

We seem to disagree on the meaning of "beta". I mean it's not stable nor finished, nor top quality. That's a choice, because i'd like to discuss the interface and ideas with the community first.

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

[The_Big_Boo]

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.

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.

[TGYoshi]

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?