importer dead

Thank, Brent, that did it.

Unfortunately, now there’s another problem. The importer is completely dead in the water.

This code starting on line 208 of AbstractLaunchAction kills it.

if (data == null) {
ErrorDialog.warning(
Resources.getString(“Error.invalid_vassal_file”),
Resources.getString(“Error.invalid_vassal_file”),
Resources.getString(“Error.invalid_vassal_file_message”,
lr.module.getAbsolutePath())
);
setWaitCursor(false);
return null;
}

It’s not obvious the best way to get around that. I’ll take a look at it briefly, but until Nov. 3, I don’t really have a lot of time.

  • M.

Post generated using Mail2Forum (mail2forum.com)

Michael,

OK, that’s good actually. It means that Vassal was not able to build a metadata object for the file you where tryng to open. Currently it can only do this for the 3 known types MODULE, SAVE and EXTENSION.

What we need to do is change AbstractMetaData and add a new metadata type to the FileType enum, and then update AbstractMetadata.buildMetaData() to recognize an ADC file and return an AdcMetaData Object.

B.
*********** REPLY SEPARATOR ***********

On 26/10/2008 at 8:04 AM Michael Kiefte wrote:

Post generated using Mail2Forum (mail2forum.com)

Okay, well, I guess it wasn’t that hard. Nothing like a good panic on a Sunday morning.

Single line edit committed to r4326. Can someone merge that for me?

  • M.

2008/10/26 Michael Kiefte <mkiefte@dal.ca (mkiefte@dal.ca)>

Post generated using Mail2Forum (mail2forum.com)

For the time being, I just jumped over that code for imported files. Is there anything in MetaData that we need for imported files? In that code, I only saw references to module-specific heap and stack sizes.

  • M.

2008/10/26 Brent Easton <b.easton@exemail.com.au (b.easton@exemail.com.au)>

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Michael Kiefte”:

Merged to trunk@4342.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Hi Michael,

The key thing here is that AbstractMetaData.buildMetaData() should be able to absolutely identify each type of file that Vassal knows about and return it’s type. For import files, the class ImportMetaData will possibly only hold the import file type, but might also hold stuff like source module name.

The reason that AbstractLaunchAction was failing is that the import file name is being set into the LaunchRequest.module field which is incorrect. This is why it is doing heap size stuff. An ADC2 import file is not a module. There are other fields in AbstractLaunchAction (game and extension) to hold a game file or an extension file name. There should be a new one called ‘import’ that holds the file name of a file to be imported.

Do you have code in the importer somewhere that can check if a given file is actually (or at least probably) a valid ADC2 import file? I can see the three suffixes you import, but can you determine that a file is actually ADC2? This idea is we catch invalid files right upfront before running them into the importer.

Cheers,
Brent.

*********** REPLY SEPARATOR ***********

On 26/10/2008 at 8:48 AM Michael Kiefte wrote:

Post generated using Mail2Forum (mail2forum.com)

Unfortunately, no. There’s no magic numbers or info headers or anything. The importer will happily attempt to import anything until the data becomes absurd at which point it will throw an exception (which is not ideal).

There’s no way to catch an invalid file until the importer gets a hold of it. For example, almost anything is acceptable in the first few bytes until something becomes internally inconsistent. Without duplicating code, there’s no really good way to determine if something is a valid ADC2 file until you’ve attempted to import it.

Actually, now that I’ve written this, there is actually a way… The format is carved up into blocks which are separated by a single block-separator byte. I could read in the first block and see if it’s reasonable. Although this is duplication of some code, the odds of a random file duplicating a valid first block are quite low.

  • M.

Post generated using Mail2Forum (mail2forum.com)

Michael,

Have a look at swampwallaby-work@ . I have built the framework you need. All you have to do is
write ADC2Utils.buildMetaData(File file).

If file is definitely not an ADC2 importable file, it should return null, otherwise it just returns a new ImportMetaData().

A the moment, ImportMetaData is just a place marker to tell the LaunchRequest it is an Importable file, it does not contain any information. I have left commented code to show where Sun Tzu and Cyberboard imports fit in, I suspect we may need to store the Import type in the metadata to make life easier on ImportAction, but it is not really needed with just the one import type.

Regards,
B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

That should be swampwallaby-work@4355


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

If I understand what you’ve got correctly, all you need to know is whether the file pointed to is one of the three types of ADC2 files that can be imported.

I wouldn’t put buildMetaData in ADC2Utils, however. buildMetaData should go in ImportAction as it knows about file name extensions (right now, the easiest way to know if it’s a valid ADC2 file – however, you can check the first byte which has to be within a certain range of values). Also ImportAction has an array called IMPORTERS which points to all of the currently valid importer classes. The parent class Importer should have a new method called buildMetaData and then ImportAction could call what it thinks is the right one based on the filename extension.

What do you think?

  • M.

Post generated using Mail2Forum (mail2forum.com)

Hi Michael,

Yes, that’s correct. You should include and easy checks that absolutely rules a file out from being an Importable file and return null. Anything else returns an ImportMetaData object.

Yes, that all sounds good - you know that end of the code better than me. I just roughed out how it should work. Adding the importFile field to LaunchRequest solved the problem we had the other day. Filename extensions is not really that good for determining if it is really and ADC2 file, plenty of applications generate .map files.

It needs to be expandable to cater for Sun Tzu and Cyberboard importers also.

I would prefer that we have just the one ImportMetaData class for all importable files, but I leave that up to you. You may need to add a field to ImportMetaData that identifies the particular importer to be used.

Cheers,
B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Hi Michael,

Did you ever get this ImportMetaData issues sorted out? I have another problem now that is related, but need your changes to the handling of Import launch requests before I can fix it.

Thanks,
Brent.

Hi Brent,

I’ll work on it tonight. I’m not sure I completely understand how it works, but I’ll figure it out.

  • M.

2008/11/17 Brent Easton <messages@forums.vassalengine.org (messages@forums.vassalengine.org)>

Post generated using Mail2Forum (mail2forum.com)

Brent,

Check out r4505 in the kiefte branch. I made a stab at generating MetaData for imported files. I stuck with the ModuleMetaData because imported modules are still basically modules. I could have subclassed it, but there’s absolutely nothing to add.

Deciding which importer to use is ever so slightly sophistocated. For example, MAP files have to begin with 0xFD. That should cut down on 99.6% of MAP files that don’t come from ADC2 ;) The other formats aren’t quite as strict.

  • M.

Post generated using Mail2Forum (mail2forum.com)

Hi Michael,

Thanks, I will incorporate that in.

Regards,
Brent.

*********** REPLY SEPARATOR ***********

On 18/11/2008 at 6:03 PM Michael Kiefte wrote:

Post generated using Mail2Forum (mail2forum.com)