Outpost Universe Forums

Projects & Development => Projects => Topic started by: Vagabond on June 03, 2017, 12:30:39 AM

Title: OP2MapImager Development
Post by: Vagabond on June 03, 2017, 12:30:39 AM
We have been working on a command line tool for rendering images of Outpost 2 Maps larger than the small thumbnail produced by the current mapper.

Some discussion has been made in the  Remote Pair Programming Thread (http://forum.outpost2.net/index.php/topic,5953.0.html?PHPSESSID=f0d6af09b0c0b720e48469de7f42f8a3). To keep from crushing the thread on Pair Programming, I'm trying to move information specifically this program's development here.

Project Goal: Create a command line program that can consume and Outpost 2 map and create a custom scaled image of the map in common file formats.

Development Milestones

Code Location: https://svn.outpostuniverse.org:8443/!/#outpost2/view/head/GameResources/OP2MapImager

Implementation Details
 * Repository Type: Subversion
 * Language: C++
 * Windows IDE: Visual Studio 2017
 * Image manipulation library: FreeImage

While Hooman and myself have been working on it so far, if anyone else if interested in helping out, we would be happy accepting the help.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 03, 2017, 12:51:45 AM
Selecting a library for image manipulation

I've found 2 major candidates so far, CImg and FreeImage.

CImg
CImg (http://cimg.eu/) is an easy library to incorporate, only requiring the addition of a single header file. They claim easy cross platform compatibility. One downside is CImg can only work with the following image formats: RAW, ASC, HDR, INR, PPM/PGM, BMP, PAN, and DLM. I would prefer to be able to output in BMP, JPG, and PNG. CImg can output in JPG and PNG, but it requires the end user to install a cross platform command line tool called Image Magick (http://www.imagemagick.org/script/index.php). Actually ImageMagick supports Unix, Windows, and Macintosh, so I guess it wouldn't work on Linux? ImageMagick's Windows download size is about 24 megabytes.

If we worked with CImg, it would probably be possible to push an error code if the user tried to export a map image to disk using PNG or JPEG telling them they have to download ImageMagick or use BMP.

FreeImage
Another option would be FreeImage (http://freeimage.sourceforge.net/). It supports Linux, Macintosh, and Windows and can work with BMP, JPG, and PNG files without another dependency like ImageMagick. However, I think you have to precompile the FreeImage code on your specific operating system before use. It would probably require some more work to function than using CImg.

.Net
The third option would be .Net. I know that Microsoft has been working to push native compiled versions of .Net for cross platform work, although I'm not sure how mature the effort is and I have a feeling it might take a lot of work and dependencies to get working over a smaller dedicated image manipulation library.



Does anyone have any experience with an image manipulation library they could recommend (or recommend against), or experience with CImg or FreeImage? The library would need to be able to:

* Create an image in BMP, JPG, and PNG
* Resize images to a smaller size
* Add small images to a larger image file (IE lay each tile's image into the larger image)
Title: Re: OP2MapImager Development
Post by: Vagabond on June 05, 2017, 01:02:42 AM
Today we settled on using FreeImage for image manipulation. It allows saving in PNG and JPG format without a third party application being installed on the end user's machine.

We wrote functions to pull cell properties on the map based on provided (X,Y) coordinates.

We managed to get FreeImage compiling and interacting with our code base. Basic code is written for saving an image file as a PNG.

Unfortunately, the tile set BMPs in Outpost 2 are not written as standard BMP files. FreeImage had difficulty loading it. The OP2 Mapper project includes code for opening the Outpost specific BMP files (written by Hooman). We didn't have time to extract the required parts yet though.

Current progress has been pushed to the repository.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 07, 2017, 10:47:16 PM
Over the last 2 days I put some more time into the OP2Imager.

In particular, I put together a first draft of code to accept and parse arguments provided as command arguments by the console.

On a side note, I learned you can add command arguments for debug sessions within Visual Studio:

I also created a library project called OP2Utility. I'm moving all the reusable code to this library so it will be available in future projects. While we could spend a lot of time building relevant code in this library, my plan is to just add the required functions to get the OP2MapImager completed. Once the OP2MapImager is completed, I'd like to create a SVN tag of the utility library and link the OP2MapImager to the tag. This way anyone in the future can modify the working copy of OP2Utility in any way they wish without breaking functionality on the OP2MapImager.

I've located all the required functions to scale and copy/paste image pixels in the FreeImage library. Now they need to be implemented in code and tested.

I'm curious how large a map image we can produce before seeing major performance issues. There should probably be some thought put into limiting the size of a map image to something reasonable. Each Outpost 2 tile is 32x32 pixels (I believe). A 512x256 tile map rendered at 100% would be 16,384 x 8,192 pixels. Not sure that is smart to let the user create an image that big, especially if a batch create is available.
Title: Re: OP2MapImager Development
Post by: Hooman on June 10, 2017, 08:03:43 AM
For reference, the NASA Blue Marble (https://visibleearth.nasa.gov/view_cat.php?categoryID=1484) site has images of the Earth at 21600x21600 resolution. The files are huge though, and take a while to download and view.

Maybe just let the user choose what they want. They'll know when it's too big.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 12, 2017, 10:46:19 AM
Hooman and I spent time working on the OP2MapImager last weekend. Most of the time was spent refactoring/fixing the code I had written through the week and developing better implementation details on how the images would be scaled by FreeImage and accepting inputs from the command line.

In particular we:

We definitely did not accomplish the primary goal of getting the BMPs into memory. However, I think we have a much better handle on how to use FreeImage to scale and create the map renders and have a better setup within the console application.

Hooman also taught me some tricks on properly handling the disposal of objects assigned to pointers and how to create destructors.

I plan on writing another post soon with how we plan to implement getting arguments from the console application. Will be looking for feedback on implementation before finalizing and coding it.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 15, 2017, 01:00:32 AM
I've been thinking more on how to accept input from the console for the OP2MapImager. This is the first legitimate console application I've helped designed, so please bear with my learning curve.

Application Assumptions
Currently I was considering something like this
Code: [Select]
OP2MapImager [OutputFileType = PNG [PNG|JPG|BMP]] [PercentScaled = 25[%]] <MapFilename.[map|OP2]>
Examples
    OP2MapImager PNG 25 Ashes.map
    OP2MapImager 50 C:\Users\UserName\Documents\Ashes.map
    OP2MapImager ..\Ashes.map
    OP2MapImager PNG Ashes.map bhsurv.map atwmon.map

After doing some thinking and reading, I would like to change the syntax. I think it would be much easier to parse the data if we use the -Short and --Long syntax for parameters. This would allow the end user to re-arrange the order of parameters at will, and simplifies the code to figure out which paramter they are placing in which order. It also seems more standardized.

Code: [Select]
OP2MapImager [-if = PNG [PNG|JPG|BMP]] [-tw = 8] filename...

USAGE:
-?  --Help
-if --ImageFormat       File format of the output image
-tw --TileWidth = 8    Integer value indicating the length of pixels in each rendered tile.
                        32 = full size. 1 = one pixel per tile.
-od --OutputDirectory   Set a different directory for outputting map renders. Create the directory if required.
-o  --Overwrite = true  Indicates if new renders will overwrite files with the same name. Will output an error if unable to filename is already taken.
-v  --Version           Software version information
My sources for Command Line Input (CLI) Syntax Documentation:

Anyways, I'm open to suggestions on changing any of the syntax.



Formatting Input for Map Scale???

Initially I was thinking of accepting a percentage value for scale. Hooman pointed out this would require handling fractional tile sizes. For example, if the user selected 17% scale, each tile would be 5.44 pixels in length. I would rather not deal with handling fractional pixel placement. So I thought we could just force round the percentage. In this case, you could either force the image to have 5 or 6 pixels per tile. This is not very precise. As a user I would expect to get an image that was exactly 17% in size if that is what they asked for.

So, what if we allow the user to set the pixel width of each tile instead of the scale. In this case you would input 5 and get a render that was 5 pixels in length per tile. There could be 32 discrete scale choices (although the bottom end at 1 or 2 or 3 pixels per tile may be rather useless). This seems cleaner to me than percentages though.

Thoughts?...
Title: Re: OP2MapImager Development
Post by: leeor_net on June 15, 2017, 04:48:21 AM
For reference, the NASA Blue Marble (https://visibleearth.nasa.gov/view_cat.php?categoryID=1484) site has images of the Earth at 21600x21600 resolution. The files are huge though, and take a while to download and view.

Side note -- it took me about 8 seconds to download one of these giant images. This seems to have more to do with an end user's broadband speed more than the size of the file. ;)

As a thought, couldn't you just have the program output the full size of the map and if the user wanted to scale it down they could do so in an image editor?

Or, you could have the utility render it full and then simply scale it down for output? Any reasonably good image library should be capable of doing this in memory.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 15, 2017, 12:07:56 PM
leeor_net,

FreeImage does contain scaling functions.

It didn't occur to me to consider scaling the entire map after rendering it. This would allow us to just accept a simple percent to scale by.

Our current plan is to initially scale the tile set images down to the proper size and then place them on a blank bmp that is created at the final render size. I think this method will reduce the memory footprint of the application and the processing time compared to what you are suggesting. Since we do not yet have the program loading the OP2 Tile Set BMPs into memory, we haven't run any tests to see the actual processing time and memory footprint yet. If the differences were not significant, perhaps it would make since to implement what you are suggesting.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 19, 2017, 02:00:15 AM
I made some progress on the OP2MapImager this weekend. I managed to find a copy of the Tile Set BMPs on this forum that had been resaved in the non-Outpost 2 specific format. I used these images to do some tests with FreeImage. In the process, I learned a lot about BMPs. Fortunately Hooman was already aware of the issues we would be facing with the render and threw out some specific terms during the last pair session that directed my efforts and research. Otherwise, not nearly as much progress would have been made.

It turns out that all the Outpost 2 tile set BMPs are 8bpp and contain palettes that index up to 256 colors for use. Each tile set contains a different set of 256 colors on their palette. FreeImage can only paste pixels from one image to another when using palettes if the two image palettes are identical. Because of this, FreeImage is unable to paste different tiles together even with the standard formatted Outpost 2 BMPs.

SIDE NOTE: The current Tile Set BMPS (well00XX.bmp) packed in maps.vol at SVN location Outpost2SVN\GameDownload\Outpost2\trunk and the primary game download (http://outpost2.net/outpost2.html) contain the old image format. If we could replace these with the non Outpost 2 specific BMP format, it would simplify things somewhat. Also, why don't we have some release log somewhere with what has been changed in Outpost 2 with each release???

Anyways, Gimp can strip palettes from a BMP and export them as 24bit BMPs. This increases their size by about 2.5 times, which is probably why Outpost 2 used the palettes in the first place?

After resaving the BMPs without palettes, FreeImage was able to scale and paste tiles from the different tile sets into the larger render! Unfortunately, it is choosing the wrong tile set and indices for about 1/4 of the tiles on the test map. The test map contains most of its tiles from well0001.bmp, which are the ones that are mostly right. I'm unsure if I'm pulling the data out of the MapData struct improperly or if the MapData struct is pulling data from the .map file improperly (or both)?

After that success, I retested FreeImage, and as long as you set the final output image as 24bits, FreeImage will paste chunks from a 8 bit palettized image without problem. I had tried testing it at 8 and 16 bpp earlier with the palettes, not really understanding what I was doing.

OP2MapImager currently scales each tile set image down, then pastes each chunk onto the final render already pre-scaled. Creating a render at scale factor of 32 (32 pixels per tile) takes about 11 seconds for a 64x256 tile map. Some diagnostic tools of Visual Studio were running in debug mode, which may have slowed it down a little bit. It looked like the bulk of time is spent actually saving the image to disc and not pasting all the tiles in. Final file size for a PNG was about 15MB.

Code has all been uploaded to the repository. It is a moderate disaster right now since I was learning and testing while working.

I wanted to upload the full size render, but the forum limits attachment sizes, so this render is 512x2048 pixels. I think that is about 25% size... Anyways, I'm happy with the results so far, but need to take a break on it for a while. Still looking for comments on how to format the console input (about 3 posts ago in this thread).

(http://forum.outpost2.net/index.php?action=dlattach;topic=5965.0;attach=804;image)
Title: Re: OP2MapImager Development
Post by: leeor_net on June 19, 2017, 01:21:24 PM
This is remarkable progress... Nice work!

Looks like some basic indexing issues to clean up an go from there.

As for switches --

My personal suggestion is to keep it as simple as possible. I would choose a lossless output format and just leave it at that (png). End user can convert it to whatever they want afterward.

I would leave scaling at something like:

Code: [Select]
-scale 1.0

Simple floating point value. 1.0 being full scale, 2.0 being double scale, 0.5 being half scale, etc.
Title: Re: OP2MapImager Development
Post by: Vagabond on June 21, 2017, 05:07:07 AM
@leeor_net,

Thanks! It sounds like you are comfortable with switches (-S / --Scale) so I will look at going with this format.


I tinkered with the code some today. Looks like I was pulling tile set indices incorrectly from MapData.h. I fixed this and managed to get a perfect render! Below is Twin Valleys in scale factor of 8 (8 pixels per tile).

Unfortunately, I get an out of range on vector access for any map that is not 64 x 256 tiles in size. Well I didn't test every size, but the several I did outside this size didn't work. I'm getting a negative tileInfoIndex value which throws the exception when accessing the TileInfo vector.

I'm guessing the function Hooman and I wrote to translate an (x,y) coordinate into an index within the 1D array may be messed up and only working with this size of map??? The code seems to throw the out of range exception at different (x,y) coordinates based on the map size. I don't have the brainpower to verify/troubleshoot bitwise operations right now (and even if I did, I may not have paid enough attention to what Hooman was teaching me to solve on my own...). Also, this doesn't seem to match up with the culprit, since it is the tileInfoIndex and not the cellIndex throwing the error.

Code: [Select]
size_t MapData::GetCellIndex(unsigned int x, unsigned int y)
{
    unsigned int lowerX = x & 0x1F; // ... 0001 1111
    unsigned int upperX = x >> 5;   // ... 1110 0000
    return (upperX * mapHeader.mapTileHeight + y) * 32 + lowerX;
}

Twin Valleys ( Scale Factor of 8 )
(http://forum.outpost2.net/index.php?action=dlattach;topic=5965.0;attach=807;image)
Title: Re: OP2MapImager Development
Post by: Vagabond on June 21, 2017, 01:39:19 PM
Quick update. The problem is associated with certain tile sets. In particular, if the map contains the yellow/orangish sand or glaciers, it throws the exception. Just happens that I got lucky and Rising from the Ashes and Hidden Valley both didn't contain these tile sets within the map itself. When looking at the vector, it looks like within the tileDataVector (list of all tiles displayed on the map) some of the tileDataIndices (the index where the tile's global information is contained with the tileInfoVector) are being set to negative. I'm assuming it is these tilesets, but I haven't dug in and confirmed the correlation. Not sure what is causing the negative values either.

Plymouth 01 ( 64 x 64 tiles, Scale Factor 8 )
(http://forum.outpost2.net/index.php?action=dlattach;topic=5965.0;attach=809;image)
Title: Re: OP2MapImager Development
Post by: Vagabond on June 22, 2017, 01:18:03 AM
Another Update: I found the problem! a couple of variables in the MapData structure needed to be set to unsigned so they could fit all the legal values. It was wrapping the values past their max value to a negative number.

Both glaciers and orange sand are rendering properly now.

I went ahead and generated a PNG image atwmon.map (512 x 256 pixels) at scale factor 32 (100%). It took 41 seconds on my computer in Visual Studio debug mode. My CPU is a little older, so a newer machine and running a final release build will see a little better results. Final PNG file size was 56,109 kb. Rendering it at a more reasonable scale factor of 4 was close to instantaneous.

I tested out JPG export and it worked fine. PNG will still be the default output though.

Now that the program is working as intended, I am considering working some minor polishes like handling overwriting files and errors. The other option would be looking at VOL decompression or cleaning up the console input arguments.

atwmon.map rendered at scale factor of 1 (1 pixel per tile)
(http://forum.outpost2.net/index.php?action=dlattach;topic=5965.0;attach=811;image)
Title: Re: OP2MapImager Development
Post by: leeor_net on June 23, 2017, 09:15:00 AM
Very nice work! I've been meaning to do something like this but never found the time to do it. So yes, you're better than I and I love it. :D
Title: Re: OP2MapImager Development
Post by: Vagabond on June 23, 2017, 06:50:15 PM
Thanks leeor.

I added bool values to allow the program to execute quietly (do not cout string to console) and overwrite (whether to overwrite a previous map render or not). The application now iterates through numbering a file to keep from overwriting. Basically the application appends _X, where X is an integer to keep from overwriting previously saved files if not desired.

All map files in a given directory may now be iterated through by inputting the directory path instead of a file path. I also cleaned up cout messages explaining what the application is performing.

I tested the greenworld tileset and the program had no problems imaging greeworld maps. See Bridge Defense below at a scale factor of 6. I also tried loading a save file (since they share very close file structure to map files). Unfortunately, all values in the map header seem to be set wrong in memory. I didn't troubleshoot beyond this.

Using FreeImage default settings, JPG files are saving at less than a quarter of the PNG sizes, which surprised me. I figured they would be fairly similar in size. I'm not sure without reviewing documentation as to how much compression is going on with the JPGs.

-Brett

Bridge Defence rendered at scale factor 6
(http://forum.outpost2.net/index.php?action=dlattach;topic=5965.0;attach=813;image)
Title: Re: OP2MapImager Development
Post by: Hooman on June 25, 2017, 12:55:08 AM
Very nice progress on this. I'm thoroughly impressed!

Nice that you caught the signed/unsigned bug.

To get saved games files to work, you'll need to skip over the saved game header. I believe it's fixed size. Read the text file in the OllyDbg folder for details on the file format. You should be able to just seek/reposition the read pointer some fixed number of bytes ahead to get past the header for saved game files, then process the data as usual. If you want to be clever about it, you can also verify the saved game file string marker before seeking ahead.

For PNG versus JPEG, the size difference is likely due to the lossy compression used by JPEG (it throws away data, and hence changes the image). I believe PNG also has lossy compression options, but the default is lossless. That's why things like text and fine lines look so much better with PNG. For natural images it's harder to tell the difference between the two. If you can generate an image with wall tiles, or anything with straight hard edges, you may notice a quality difference between the two formats.

When appending numbers to filenames to avoid overwriting, maybe append the scale factor, or final dimensions. That's the only real input the should change the output, other than actually modifying the source map or tilesets. Considering the nature of the transformation, you probably don't need to worry too much about protecting from overwrites.

Using named switches might make the command line parameter processing cleaner. Remember to consider what happens if a filename just happens to coincide with a switch name. ;)
Title: Re: OP2MapImager Development
Post by: Vagabond on June 25, 2017, 12:42:59 PM
As alluded in the other post, OP2MapImager can now take renders of save files. It is a bit silly though since it doesn't include any of the building or units, just the background terrain. So you could just take a picture of the underlying map and get the same results. There could be a fun project involved in parsing out all the information in a saved file and rendering it so you could capture a huge battle or share a high def render of your utopian colony layout. I think this will remain outside the scope of the project for me though.

I finished implementing standardized switch statements. Still some testing to work through though.

Quote
When appending numbers to filenames to avoid overwriting, maybe append the scale factor, or final dimensions. That's the only real input the should change the output, other than actually modifying the source map or tilesets. Considering the nature of the transformation, you probably don't need to worry too much about protecting from overwrites.

Since there is both a grnwld01.map and grnwld01.bmp (tile set), without file overwrite protection, it is entirely possible to make some interesting mistakes. Fortunately, the stock tilesets don't suffer from it.

I'd like to finalize the big picture of the project. Currently, there are 2 projects, OP2Utility and OP2MapImager. OP2MapImager is the actual console application. OP2Utility contains XFile (eventual cross platform file system support) and MapData struct.



OP2MapImager Reusability Concerns

Is anyone possibly considering reusing parts of this code in other projects? If so, what would they/you prefer structure wise from my end.

Option A would be moving the render code over to OP2Utility so anyone could call it and render a map from another project. However, it will carry the dependency to FreeImage into the dependency project, when everything else in the library does not require FreeImage.

Option B would be a third library project that just contains the FreeImage dependency and render code. This way other projects could reference OP2Utility and not worry about FreeImage, or you could reference both library projects if needed. The downside is you now have to hook in yet another library project.

Option C would be to leave all the render code in the console application. I think you could just call the console application in quiet mode from another application to render a map.

Anyways, I'm willing to work any of the three ideas, just curious if there are opinions to weigh in on before I dive in coding.



Render of a save file, MesaMissions (a scenario I've been working off and on for a while)
(http://forum.outpost2.net/index.php?action=dlattach;topic=5965.0;attach=815;image)
Title: Re: OP2MapImager Development
Post by: Hooman on June 26, 2017, 02:28:12 AM
Quote
Since there is both a grnwld01.map and grnwld01.bmp (tile set), without file overwrite protection, it is entirely possible to make some interesting mistakes. Fortunately, the stock tilesets don't suffer from it.

That's one of the reason why I suggested appending an output parameter, such as the scale factor:
grnwld01.01.bmp
grnwld01.32.bmp

People would have to go out of their way to have a problem, which I think is good enough.


Project organization can be a pain. Maybe just move the main method out into a Main.cpp, and let the rest implicitly be library like code. It might not be separated out into a library, but someone could easily cannibalise the project or make such changes if they wanted to.
Title: Re: OP2MapImager Development
Post by: leeor_net on June 26, 2017, 11:29:07 AM
I think Hooman pretty well covered it -- PNG is lossless, JPEG isn't. Personally I hate JPEG. PNG can be squashed some using external tools like pngcrush which removes superfluous data.
Title: Re: OP2MapImager Development
Post by: Hooman on June 27, 2017, 03:06:29 AM
I was just doing some reading on this and found some interesting stuff. Seems there are lossy pre-processors that take PNG encoding into consideration. It modifies the image before being saved as PNG, so the compressor is able to handle it better and compress to smaller images. The actual PNG encoding process itself is still loseless, but the pre-rprocessing step done before saving is lossy. That would make it pretty similar to JPEG, though with a 2-step process.

On a related note, if you do many edit/save cycles with a lossy encoder, there is generational loss (https://en.wikipedia.org/wiki/Generation_loss). For a video: JPEG generation loss (https://www.youtube.com/watch?v=jjhomJ04S18).
Title: Re: OP2MapImager Development
Post by: Vagabond on June 28, 2017, 05:05:39 AM
Hooman and leeor,

Thanks for the input. I'll just plan to leave the render specific code in the console application for now. I'm working through adding VOL decompression right now.

Hooman, interesting video of the JPEG generational loss. Good to think about when choosing a format for a master image or sound file.

-Brett
Title: Re: OP2MapImager Development
Post by: Vagabond on June 29, 2017, 11:01:33 PM
Another update,

I have managed to hook Voldecompress code into the MapImager. I fed the code Ply01.map and after not finding the file loose in the directory, the application extracted it from the proper vol file and rendered it.

I added a switch that allows toggling ArchiveAccess. It defaults to true if not included.

I copy and pasted the Voldecompress code into the OP2Utility project under a folder called Archives. As vaguely alluded in the pair programming thread, I was looking at making some changes to Voldecompress.

* Wrapping the Voldecompress library code into  a namespace called Archives. This way it isn't present in the global namespace by default when including OP2Utility. Also, it would help add clarity on what the classes are used for when seen in code.

* Removing the C prefix from all the class names and filenames.

So, you would call Archives::Volfile(filename).

* Adding a function that allows pulling a copy of an archived file into memory without saving it to disk. Currently OP2Mapper saves the map file to disk then opens it. It might be nice if it didn't have to Save the file first.

Thoughts on these changes?

I pretty happy with the progress so far.
Title: Re: OP2MapImager Development
Post by: Hooman on June 30, 2017, 02:44:09 AM
Quote
* Adding a function that allows pulling a copy of an archived file into memory without saving it to disk. Currently OP2Mapper saves the map file to disk then opens it. It might be nice if it didn't have to Save the file first.
That actually surprises me a little. The backend was written to support direct loading from VOL files. Though the frontend might just lack the interface to allow that. Guess I didn't really play around with the map editor enough.  ???
Title: Re: OP2MapImager Development
Post by: Vagabond on July 01, 2017, 10:58:32 PM
@Hooman,

I created a separate thread to discuss modifying the VolDecompress code here: http://forum.outpost2.net/index.php/topic,5973.0.html

I just finished a draft of the OP2MapImager ReadMe file. See below. If anyone notices any grammer issues or other suggestions, please let me know. I've been putting OP2MapImager through its paces, tyring out different combinations of commands and cleaning the code some. There is still a fair amount of work to do cleaning the code (I'm not a great C++ developer either, so clean to me will probably not be clean to someone more experienced).




OP2MapImager Ver 1.0 - Outpost 2 Map and Saved Game Imager
Developed by Hooman and Brett208 (Vagabond)

+++ USAGE NOTES +++
  * Capable of saving multiple map files and entire directories.
  * The OP2MapImager executable and FreeImage.dll must be in the same directory as the tileset BMPs.
  * Scale Factor (-s) determines the final render size and represents the final pixel length of a single tile
    * Max Value: 32, renders at full size, or 32 pixels per tile.
    * Min Value: 1, renders at 1 pixel per tile

+++ EXAMPLE COMMANDS +++
  * OP2MapImager mapFilename.[map|OP2]
  * OP2MapImager -s 16 -o -q Ashes.map eden01.map sgame0.op2
  * OP2MapImager --Scale 8 --ImageFormat BMP [Directory of choice]

+++ OPTIONAL ARGUMENTS +++
  -H / --Help: Displays Help File
  -Q / --Quiet: [Default false] Add switch to run application without issuing console messages.
  -O / --Overwrite: [Default false] Add switch to allow application to overwrite existing files.
  -D / --DestinationDirectory: [Default MapRenders]. Add switch and name of new destination path.
  -I / --ImageFormat: [Default PNG]. Allows PNG|JPG|BMP. Sets the image format of the final render.
  -S / --Scale: [Default 4] Sets Scale Factor of image.
  -A / --AccessArchives [Default true]. Add switch to disable searching VOL archives for map and well files.

For more information about Outpost 2 visit the Outpost Universe (http://outpost2.net/)
Image Manipulation accomplished through FreeImage (http://freeimage.sourceforge.net/)


+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +


+++ INSTALL DIRECTIONS +++

Ensure OP2MapImager.exe, FreeImage.dll, and well0000.bmp through well0012.bmp are all in the same directory. (I typically put them all in the root OP2 install directory for ease of use, but not required.)

Open a command prompt. Navigate to the directory with MapImager.exe and type 'OP2MapImager -h'.


+++ OUTPOST 2 TILESETS (WELL00XX.BMP) +++

The original tileset images shipped with Outpost 2 use a special format that prevents them from being opened by typical pixel image manipulation software (like Gimp or Paint). However, Outpost 2 is capable of reading normally formatted 8bpp indexed BMPs.

OP2MapImager contains a copy of the tileset images that have been reformatted from the OP2 specific format to general BMP format. These may be placed in the root directory of Outpost 2 if you wish. They must exist in the same directory as OP2MapImager.exe and FreeImage.dll.

If you attempt to render a map using the original Outpost 2 tileset images, OP2MapImager will throw an error.


+++ ARCHIVE (.VOL) FILE ACCESS +++

By default, OP2MapImager is able to search through .vol archives and pull maps or tilesets out of the .vol files. OP2MapImager currently leaves the .map file or tileset image file loose in the directory after pulling the file. These loose copies may be deleted manually by the user without affecting the ability of Outpost 2 to run.

OP2MapImager will first search the supplied directory, and if it cannot find the file of interest, then it will search alphabetically through all .vol files in the directory for the given file. So if the file exists both loosely in the directory and in a .vol file, the copy not in the .vol file will be used.


+++ RELEASE COMPILATION INSTRUCTIONS +++

Source code may be found at: https://svn.outpostuniverse.org:8443/!/#outpost2/view/head/GameResources/OP2MapImager.

 1. Set Solution Configuration to Release.
 2. Set Solution Platform as desired. (x86/x64).
 3. Compile Code.
 4. Determine version number for this release.
 5. Grab the following files, and place them in a zipped directory with name format 'OP2MapImager 1.0 x64':
    * OP2MapImager.exe (Release Directory)
   * FreeImage.dll (There are different DLLs for x32 and x64 compilations)
   * Well0000.BMP-Well0012.BMP (Ensure they are the reformated versions that a normal image editor may open.)
   * ReadMe.txt (this file)
   * FreeImage liscense-gplv3.txt
 6. Place the following files in a separate zipped directory with name format 'OP2MapImager 1.0 x64 Debug':
    * OP2MapRender.pdb
   * OP2Utility.pdb
 7. Place both zip files on the Outpost Universe Website.

Title: Re: OP2MapImager Development
Post by: Hooman on July 02, 2017, 06:35:44 AM
Nice job with the documentation.

We'll have to get the custom OP2-format BMP loading code working, as well as loading directly from VOL files. I consider the direct VOL loading to be higher priority, since you'd need to read the VOL files before getting access to the custom format BMP files stored inside. Of course that could just be me procrastinating on touching the messy custom format BMP loading code.

The release build steps seem long and manual. Perhaps we should script that. Maybe we could create a new project in the solution space for building the release package. It could depend on the project, so building it would automatically rebuild the project, and fail if the project failed to build. Maybe throw the commands into the post build step to copy files to the right folder and zip them up. I don't know how you want to handle version numbers, though it would be easy enough to append a date to the archive name.
Title: Re: OP2MapImager Development
Post by: Vagabond on July 03, 2017, 03:50:22 PM
Hooman,

I spent some time learning more about post build events in Visual Studio. Below is the result. Now Visual Studio will check if the code was built in release mode, and if so create a folder called 'OP2MapImager X.X' inside the release folder. Then it populates the new folder with all the files required for the build.

The user will still have to manually change the version number in the code, folder name, and ReadMe before zipping the folder. So, I guess it eliminates some of manual labor.

Thank you for the reply in the other topic on expanding VolDecompress. I haven't had time to implement it yet, but should be next on the list to look at. I would prefer moving away from the WIN API code, but I'm not sure it makes sense for me to just rewrite a smattering of the functions this way and leave the rest with WIN API and COM interoperability.

x86 vs x64 compatibility
I think the WIN API code used in OP2VolDecompress is not compatible with x64 builds. I can only seem to get it to compile in x86.

Code: [Select]
xcopy /y /d "$(ProjectDir)FreeImage\FreeImage.dll" "$(OutDir)"

if $(ConfigurationName) == Release (
    mkdir "$(OutDir)OP2MapImager X.X"
    xcopy /y /d "$(ProjectDir)well00*.bmp" "$(OutDir)OP2MapImager X.X"
    xcopy /y /d "$(ProjectDir)FreeImage license-gplv3.txt" "$(OutDir)OP2MapImager X.X"
    xcopy /y /d "$(ProjectDir)ReadMe.txt" "$(OutDir)OP2MapImager X.X"
    xcopy /y /d "$(OutDir)FreeImage.dll" "$(OutDir)OP2MapImager X.X"
    xcopy /y /d "$(OutDir)$(TargetFileName)" "$(OutDir)OP2MapImager X.X"
)
Title: Re: OP2MapImager Development
Post by: Hooman on July 05, 2017, 06:39:00 AM
For the X.X, perhaps your script can set a shell variable using the "svn info" command. The command on it's own will list a number of values, but you can select one specifically with the --show-item argument. I would recommend experimenting with the "last-changed-revision" value. You could tag each release with the SVN revision number. I believe by setting the path/url to the root of your project, it will include the most recent revision change to that project (ignoring more recent repository changes outside of your project). Of course if you depend on another project, it might be easier to just use "revision" (the global value). Another possibly useful value is "last-changed-date".

Code: [Select]
info: Display information about a local or remote item.
usage: info [TARGET[@REV]...]

  Print information about each TARGET (default: '.').
  TARGET may be either a working-copy path or URL.  If specified, REV
  determines in which revision the target is first looked up.

  With --show-item, print only the value of one item of information
  about TARGET. One of the following items can be selected:
     kind                  the kind of TARGET
     url                   the URL of TARGET in the repository
     relative-url          the repository-relative URL
     repos-root-url        the repository root URL
     repos-uuid            the repository UUID
     revision              the revision of TARGET (defaults to BASE
                           for working copy paths and HEAD for URLs)
     last-changed-revision the most recent revision in which TARGET
                           was changed
     last-changed-date     the date of the last-changed revision
     last-changed-author   the author of the last-changed revision
     wc-root               the root of TARGET's working copy

...

Once you have some kind of version tag, I would also recommend zipping the package as part of the build step.


If the release package preparation is slow, it may be nice to have it separated out. Looks like you're only doing it for release mode, which should be good enough. If later you decide to add testing, which might also be done in release mode, the packaging could be move to a later step. That's why I suggested a separate release package project that depends on the actual project. The actual project would still be the default project for builds, so the release package build would only run when requested, and project dependency rules would ensure the actual build was also up-to-date when it runs. Not sure if this makes sense to do; just wanted to be clear on the idea.


The Win API code does have differences between 32-bit and 64-bit. Mostly though, it uses auto-expanding type names, so they're the appropriate type on each platform. Depending on how things are coded, it can often be source compatible between the two, though not likely to be binary compatible.

You shouldn't need to use the Win API. You can depend on the standard C++ library. It's mostly just file access code that depends on the Win API, which there is reasonably straightforward replacements to in the standard C++ library.

I didn't see an easy way to do binary memory stream in the standard C++ library. It may make sense to still use your own StreamReader classes. A custom FileStreamReader class can just delegate to standard C++ library file access classes. A custom MemorySreamReader class can just be built in raw C++ without any standard library or operating specific code.

The existing code for OP2Editor used the Win API for the FileStreamReader. The MemoryStreamReader was simple C++ code without use of libraries or the Win API. The code was quite simple in both cases.
Title: Re: OP2MapImager Development
Post by: Vagabond on July 08, 2017, 05:29:19 AM
Another Update,

I spent some time learning more about the command prompt and writing batch programs, which is essentially what Visual Studio C++ Post Build Events are.

There is not a built in zip console command in Windows (strange I thought), so I settled on 7-Zip's console application. 7Zip is a fairly well known, free, and easy to use alternative. I placed 7za.exe in the repository, which means that others will be able to run the post build event without having to fetch the application manually (if they are running windows).

I had to download the console application portion of TortoiseSVN. Unfortunately, it does not default to installing the console application when you install TortoiseSVN. So, if others pull the source code, the post build script will blow up until they do the same if not already installed. Perhaps there is a way to place the executable in folder like I did with 7-zip?

The code below is the full post build script. Basically it


Code: [Select]
xcopy /y /d "$(ProjectDir)FreeImage\FreeImage.dll" "$(OutDir)"

REM SET zipName="OP2MapImager Ver1.0" svn info --show-item revision --no-newline
SET zipName="OP2MapImager Ver1.0"
SET directoryName=$(OutDir)%25zipName%25

if $(ConfigurationName) == Release (
    mkdir %25directoryName%25
    xcopy /y /d "$(ProjectDir)well00*.bmp" %25directoryName%25
    xcopy /y /d "$(ProjectDir)FreeImage license-gplv3.txt" %25directoryName%25
    xcopy /y /d "$(ProjectDir)ReadMe.txt" %25directoryName%25
    xcopy /y /d "$(OutDir)FreeImage.dll" %25directoryName%25
    xcopy /y /d "$(OutDir)$(TargetFileName)" %25directoryName%25

    7za a -tzip $(OutDir)%25zipName%25.zip %25directoryName%25
    REM DEL /q %25directoryName%25\*.*
    RMDIR %25directoryName%25 /s /q
)

The only piece I need help on is setting the revision number from SVN. I cannot figure out how to set the contents of a local variable as a concatenation of a string and the output of the SVN console command.

Successfully working SVN console command:
Code: [Select]
svn info --show-item revision --no-newline

I want it to read something like:
Code: [Select]
SET zipName="OP2MapImager Ver1.0" svn info --show-item revision --no-newline


@Hooman,

Thanks for the reply in the other thread on vol decompression. Now that I'm done fiddling with the post build event (besides the issue noted), I'll head back over and try to implement the tips you gave.
Title: Re: OP2MapImager Development
Post by: Hooman on July 09, 2017, 12:00:06 PM
This may be relevant:
https://stackoverflow.com/questions/17546016/how-can-you-zip-or-unzip-from-the-command-prompt-using-only-windows-built-in-ca (https://stackoverflow.com/questions/17546016/how-can-you-zip-or-unzip-from-the-command-prompt-using-only-windows-built-in-ca)
It seems there is a way to compress files to ZIP using Power Shell with newer versions of Windows. Power Shell can be called from a regular shell or batch file. I noticed an alternate methods towards the end that may work for older versions of Windows.

For the variable, this might help:
https://stackoverflow.com/questions/2323292/windows-batch-assign-output-of-a-program-to-a-variable (https://stackoverflow.com/questions/2323292/windows-batch-assign-output-of-a-program-to-a-variable)
Looks a bit sick, but seems to work.


On Linux, you could just use either of the following:
Code: [Select]
export variableName="Some fixed string: `command args`"

Code: [Select]
export variableName="Some fixed string: $(command args)"
Title: Re: OP2MapImager Development
Post by: Vagabond on July 11, 2017, 02:38:49 AM
While testing ways to get the directory name to display correctly with the SVN version number, I managed to set the directory to the current directory and deleted the majority of the OP2MapImager code.

Fortunately, it is all backed up to the repository and was quick to restore. (It may have all been in the recycle bin on the CPU as well, but didn't really check).

After that, I bought a book on the Windows Console and batch scripting. Currently working through the book's content. I figured it was time to actually learn what I am doing instead of hacking it constantly for post build events.

Unfortunately, the project is languishing a little as I could have just spent 2 minutes copying files over for a release instead of all these hours learning a new skill. It is something I've wanted to learn for a while though.

I don't think the subject is too complicated, so hopefully in a couple of days I'll have moved through the book far enough to know how loops, escape characters, etc. actually work and do things right.
Title: Re: OP2MapImager Development
Post by: Hooman on July 11, 2017, 09:55:40 PM
Quote
While testing ways to get the directory name to display correctly with the SVN version number, I managed to set the directory to the current directory and deleted the majority of the OP2MapImager code.
Hah! :P

Quote
Unfortunately, the project is languishing a little as I could have just spent 2 minutes copying files over for a release instead of all these hours learning a new skill. It is something I've wanted to learn for a while though.
Welcome to the world of software development. Where people spends hours or days, to save a few minutes or seconds.  ::)

Funny how that works out too. You notice some boring repetitive task that keeps eating away at a few minutes, plus the associated cognitive load. So you figure you'll spend an extra two minutes to automate the two minute task. And it works, mostly, and saves time, but there is one small little largely inconsequential detail, and that's suddenly where you sink all your time and effort to get that last little bit done that keeps eating away at you. And you wonder out loud if it was worth the time, as you've now sunk more time into automating the task than the original task ever would have cost you in the long run. Yet you still feel victorious when you finally accomplish the task, because the hidden cost of that cognitive load is no longer weighing on you. And that makes it all worth it!  :)
Title: Re: OP2MapImager Development
Post by: Vagabond on July 15, 2017, 05:12:35 AM
I finished the first of 3 books on the Windows command line/batch scripting. I figured out enough to pull in the SVN revision number and append it to the end of the directory name. The author separated his book into 3 volumes about 85 pages each. They sell for $3.00 each on Amazon for an electronic copy. I would have been willing to pay $9.00 for the book all together. https://www.amazon.com/Learn-Command-Line-Batch-Script-ebook/dp/B00V9D3QVM/

New post build script is below. New release workflow at the bottom of the ReadMe is posted below that. 

Code: [Select]
REM Place FreeImage DLL into debug or release directory for running application within Visual Studio.
xcopy /y /d "$(ProjectDir)FreeImage\FreeImage.dll" "$(OutDir)"

REM Pull repository revision number and set into final release directory name.
FOR /f %25%25i IN ('svn info --show-item revision --no-newline') DO SET svnVersion=%25%25i

SET zipNameBegining="OP2MapImager Ver1.0."
SET zipName=%25zipNameBegining%25%25svnVersion%25
SET directoryName=$(OutDir)%25zipName%25

if $(ConfigurationName) == Release (
    mkdir %25directoryName%25
    xcopy /y /d "$(ProjectDir)well00*.bmp" %25directoryName%25
    xcopy /y /d "$(ProjectDir)FreeImage license-gplv3.txt" %25directoryName%25
    xcopy /y /d "$(ProjectDir)ReadMe.txt" %25directoryName%25
    xcopy /y /d "$(OutDir)FreeImage.dll" %25directoryName%25
    xcopy /y /d "$(OutDir)$(TargetFileName)" %25directoryName%25

    REM Use 7Zip to zip final version
    7za a -tzip $(OutDir)%25zipName%25.zip %25directoryName%25
   
    DEL /q %25directoryName%25\*.*
    RMDIR %25directoryName%25 /s /q
)

Code: [Select]
 1. If changing Major/Minor revision number, set new version number at top of OP2MapImager main.cpp AND in post build event batch script.
 2. Run SVN Commit and then SVN Update to merge committed changes and properly update revision number of repository.
 3. Set Solution Configuration to Release.
 4. Set Solution Platform to x86. (x64 is not currently supported due to some WINAPI code within OP2Utility VolDecompress).
 5. Compile Code.
 6. The following files will automatically be copied into the zipped directory'OP2MapImager 1.0.XXXX' (XXXX is svn revision number):
    * OP2MapImager.exe (From Release Directory)
    * FreeImage.dll (x32 version)
    * Well0000.BMP-Well0012.BMP (Reformated BMPs that a normal image editor may open.)
    * ReadMe.txt (this file)
    * FreeImage liscense-gplv3.txt
 7. Place zip file on the Outpost Universe Website.
Title: Re: OP2MapImager Development
Post by: Hooman on July 15, 2017, 09:21:15 PM
Nicely done. You're really going all in on this.

One small point, the "x.y.z" version numbers are generally referred to as Semantic Versioning (http://semver.org/). They have implied meanings concerning compatibility between versions. The revision numbers returned by SVN are more just a serial number. It might be better not to mix the two formats. Maybe just tag it as "rev-#". If you want something more human friendly, consider appending the revision date. You could append both a revision number and a revision date.

To get proper semantic version numbers, they would probably need to be manually tagged. One way is to detect a build from a tag branch, and extract the tag name. That only works for tagged releases though. I'd say just stick with revision numbers or dates for now.


I like that you actually bought a book to learn that stuff.
+1
Title: Re: OP2MapImager Development
Post by: leeor_net on July 18, 2017, 09:11:10 AM

Implementation Details
 * Repository Type: Subversion
 * Language: C++
 * Windows IDE: Visual Studio 2017
 * Image manipulation library???

DevIL (http://openil.sourceforge.net/)

Have been using it with great success in Rogue Arena (https://github.com/ldicker83/rogue-arena).
Title: Re: OP2MapImager Development
Post by: Vagabond on July 18, 2017, 07:07:01 PM
leeor_net,

We have had pretty good success using FreeImage. At this point, all the image manipulation is working properly, so I don't see any reason to switch, although I appreciate the suggestion. I hadn't updated the initial page to indicate we had went with it.

Hooman,

I read through the article on Semantic Versioning. It makes sense to me. I'll revisit this once I get the archive code to compile again (I'll post the errors in the VolDecompress thread).

I was thinking if there was a way to load the contents of a .txt file into a compile time constant, you could specify the version number in a text file. Then you could have the C++ program load the text file for indicating it's version number. It would be trivial to load the text file into the post build event and indicate it in the zipped filename and directory name. A build number within the version number could even be incremented within the text file via the post build event. Then whenever you were ready to change the version number, you would just need to change the .txt file and it would reflect in both the project and the post build event.

-Brett
Title: Re: OP2MapImager Development
Post by: leeor_net on July 19, 2017, 02:21:19 AM
leeor_net,

We have had pretty good success using FreeImage. At this point, all the image manipulation is working properly, so I don't see any reason to switch, although I appreciate the suggestion. I hadn't updated the initial page to indicate we had went with it.

As always I'm late to the party. FreeImage is a good option too, I just happen to have inherited it with the Rogue Arena project and learned that it's quite good at what it does.
Title: Re: OP2MapImager Development
Post by: Vagabond on July 24, 2017, 09:13:46 PM
As indicated in the release post, I finally finished the OP2MapImager. Check it out here: http://forum.outpost2.net/index.php/topic,5980.0.html

I already released the first patch (ver 1.0.1). When attempting to render all maps in the directory, I realized it was pulling SGAME10.OP2, which isn't a saved game but an internal script. This would of course crash the OP2MapImager. It was also pulling and imaging wellpallet.map even though this really isn't a map.

I added two flags to the ResourceManager class. They both default to false. One allows returning OP2SGAME10.OP2 on a directory wide search and the other allows returning wellpallet.map.

I also stopped the ResourceManager from returning duplicate files if the file exists both loosely and in an archive.

There is probably room for some sort of wildcard/regex/blob support for returning files through the ResourceManager, but for now, it is suiting the OP2MapImager's purpose, so I'm going to leave it alone.

If anyone else runs into bugs, please let me know.

Sorry, it is Windows only at this time (although it might work through Wine). I would like to remove the Windows dependencies within the Archive code someday, but this would require completing a console application that acted on creating/scanning archive files to test changes first.

My knowledge of C++ has grown a lot, although it is still not great. I'm learning C++ is a tough language to learn well.
Title: Re: OP2MapImager Development
Post by: leeor_net on July 24, 2017, 09:28:25 PM
My knowledge of C++ has grown a lot, although it is still not great. I'm learning C++ is a tough language to learn well.

That is an understatement and a half! I love C++ but it's, as you said, difficult to learn to use well especially with it's annoying mix of four languages in one (C, C with Objects, Templates and STL).
Title: Re: OP2MapImager Development
Post by: Hooman on July 25, 2017, 05:00:46 AM
Quote
I added two flags to the ResourceManager class. They both default to false. One allows returning OP2SGAME10.OP2 on a directory wide search and the other allows returning wellpallet.map.

I think you might be better served by adding appropriate error handling, rather than flags to avoid special case exceptions.


As for platform compatibility, I noticed one small thing that I submitted a fix for. Use forward slash as your path separator whenever possible. Forward slash is accepted as a path separator by both the Linux kernel and the Windows kernel. In particular, this applies to the #includes.

This might sound confusing, since the Windows shell (CMD.EXE) only accepts the back slash as a path separator. Under the hood though, the Windows kernel accepts either back slash or forward slash. In the case of the C++ compiler the #include leads to a call to open a file, using the Windows kernel. Hence, either separator is acceptable here.

On Linux, only a forward slash is accepted as a path separator. That's true of both the kernel, and the shell.

In short, if you're not writing a batch file, or a command that will be run by the shell, use forward slash. It will work everywhere.


Next task: removing the dependence on windows.h. I think you're not far from that. Perhaps a future pair programming session? Maybe also have Visual Studio generate a Makefile, and then switch over to Linux and see what happens there.
Title: Re: OP2MapImager Development
Post by: Vagabond on July 25, 2017, 07:48:06 AM
Quote
I think you might be better served by adding appropriate error handling, rather than flags to avoid special case exceptions.

Perhaps it would be better to add 2 public functions to ResourceManager, one that returns all map files in the directory and the other to return all save files in the directory? These respective functions could then remove wellpallet.map or OP2SGAME10.OP2 before returning their values. Currently ResourceManager allows returning all files with a given extension, which is what is creating the problem. It might not be apparent to the user that they will not get wellpallet.map when trying to get all the files of a given extension if they don't see the flags when scanning the header file.

Another aspect that I wasn't sure about was returning filenames vs StreamReaders. I have almost 100 unique maps in my Outpost 2 copy. It seemed strange to pass out 100 StreamReaders simultaneously, so I elected to pass out filenames if you ask for more than a single file. Maybe hundreds of StreamReaders isn't a big deal though? It could cause problems though if someone in the future is using the ResourceManager to get listings of files in archives for modification I suppose.

So, I could remove the it could look like:
Code: [Select]
SeekableStreamReader* getResourceStream(const string& filename, bool accessArchives = true);

void getAllFilenamesOfType(vector<string>& filenamesOut, const string& directory, const string& extension, bool accessArchives = true);

void getAllSavedGames(vector<string>& filenamesOut, const string& directory);

void getAllMaps(vector<string>& filenamesOut, const string& directory, bool accessArchives = true);

Currently there is a public function in ResourceManager:
Code: [Select]
void getAllStreamsOfFileType(vector<SeekableStreamReader*> seekableStreamReadersOut, const string& directory, const string& extension, bool accessArchives = true);

I think it would be best to just remove this and have the user just get filenames and ask for each stream one file at a time. I'm currently not using the function in the MapImager anyways.

Quote
In short, if you're not writing a batch file, or a command that will be run by the shell, use forward slash. It will work everywhere
Thanks for fixing. I'll start to incorporate this into the future.

Quote
Perhaps a future pair programming session?
I'm interested, but it will be a month or so before it is something I would be able to dedicate a block of time to.
Title: Re: OP2MapImager Development
Post by: Hooman on July 25, 2017, 12:17:47 PM
Quote
Perhaps it would be better to add 2 public functions to ResourceManager, one that returns all map files in the directory and the other to return all save files in the directory?

That seems a bit too specific for ResourceManager. Maybe.

It might be better to offer a shell glob search function from ResourceManager. Then users can search for "*.map", "*.OP2", or even better "SGAME0?.OP2". The last search pattern would save you the trouble of one of those flags.

As for the "wellpallet.map" file, does it actually crash, or just produce an ugly unexpected map? If it crashes, you need better exception handling. I think the map imager should just produce an error line stating a file couldn't be processed, maybe some details about the error, and then continue on.

On another note, "wellpallet.map" should be "wellpalette.map". And if it's not a real map file, and can't be processed as such, maybe it should be renamed to have a different extension. What does it even do anyway? There is already tile group info in the editor, so I assume it's not just duplicating that.


Quote
Another aspect that I wasn't sure about was returning filenames vs StreamReaders. I have almost 100 unique maps in my Outpost 2 copy. It seemed strange to pass out 100 StreamReaders simultaneously, so I elected to pass out filenames if you ask for more than a single file. Maybe hundreds of StreamReaders isn't a big deal though? It could cause problems though if someone in the future is using the ResourceManager to get listings of files in archives for modification I suppose.

Open files is a finite system resource. Don't just return hundreds of open files without a very good reason. It might not even work if the file limit is set too low. I'd say dump that code, as programmers should not be encouraged to do things that way.

This limit doesn't so much apply to the memory streams, or archive file contents, since they all share one actual file, but it still seems like a bad pattern.


I'm thinking you should avoid the out parameters. Not sure if this design is about syntax, or efficiency.

Assuming efficiency, you do want to avoid copying large structs and arrays around, but a vector is really just a small data type that references an array. It should be possible to pass the vector type by value, and still get by reference transfer of the underlying array data.

Relevant: Return Value Optimization (RVO)

In regards to syntax, I think it would be more clear to simply return the vector. The function user could then declare a variable and assign it to the result of the function, rather than having to declare the variable up front to be passed in.
Code: [Select]
vector<type> localVar = functionReturningVector();
Title: Re: OP2MapImager Development
Post by: Vagabond on July 26, 2017, 08:09:24 AM
I implemented a function called getAllFilenames that accepts a string that is turned into a regex search for the filename. This gave me fine grained control to avoid the last saved game and anyone in the future to do similar things without bloating the number of functions available. I also deleted the flags for the specific filenames.

I was looking at wellpallet.map, and it isn't being distributed by the base download of the game, so it shouldn't typically be a problem. I'm not sure where/when it got added to my copy of Outpost 2. It does render into an image without throwing an error. I moved the for loop that checks for it and deletes it into the MapImager code so ResourceManager doesn't have to worry about it. Something that didn't really need to be worried about in the first place, but it is taken care of now.

Here are the public members of ResourceManager now:
Code: [Select]
SeekableStreamReader* getResourceStream(const string& filename, bool accessArchives = true);

void getAllFilenames(vector<string>& filenamesOut, const string& directory, const string& filenameRegexStr, bool accessArcives = true);
void getAllFilenamesOfType(vector<string>& filenamesOut, const string& directory, const string& extension, bool accessArchives = true);

// Searches .vol and .clm archives for file and then extracts it.
// returns true if EITHER the file is extracted OR
//     if BOTH overwrite == false AND the file already exists in the directory.
bool extractFile(const string& filename, bool overwrite = false);

// Searches all .vol and .clm files and extracts any file with the given extension.
void extractAllOfFileType(const string& directory, const string& extension, bool overwrite = false);

Quote
I'm thinking you should avoid the out parameters. Not sure if this design is about syntax, or efficiency.

I am trying to return a reference to the vector since it doesn't make since to me to copy a class in this instance. I could return a pointer, but then have to explicitly delete the object later. This way I just create it upfront and don't have to delete it.

I read up some on the Named Return Value Optimization. So basically C++ will often compile into returning a reference even though the specification is to return a copy of a class from a function? Kind of messed up???

Quote
This limit doesn't so much apply to the memory streams, or archive file contents, since they all share one actual file, but it still seems like a bad pattern.

Agreed, I deleted the function.
Title: Re: OP2MapImager Development
Post by: Hooman on July 28, 2017, 03:18:10 AM
Quote
it doesn't make since to me to copy a class in this instance
Why?

The copy here should be cheap. If you return an object not by pointer, you don't need to worry about deleting it later.


Seems like getAllFilenamesOfType isn't sufficiently different from getAllFilenames. The difference seems to amount to "type" versus "*.type".

What is the directory parameter for? I find that unintuitive.


As for RVO, how else could a function return an object? How is it transferred between stack frames? It can't return first, and then copy, since the local variables must be destroyed before returning, so there would be nothing left to copy. The only real solution is to construct the object on the caller's stack in the first place, while the callee is still active. That requires a hidden reference pointer to be passed in.
Title: Re: OP2MapImager Development
Post by: Vagabond on July 30, 2017, 01:17:05 PM
Took the time to refactor all the OUT function variable references to instead be function return values in the ResourceManager class, main OP2MapImager file, and XFile class. Everything appears to be working properly.

Quote
What is the directory parameter for? I find that unintuitive.

XFile needs to know which directory you want to search for files in given either the file's extension or a REGEX string representing different possible filenames.

I could eliminate the directory parameter from both functions and assume ResourceManager only searches the directory that contains the pre-loaded archive files. I think it makes sense to be able to search other directories for files without having to create a new instance of ResourceManager.

Quote
Seems like getAllFilenamesOfType isn't sufficiently different from getAllFilenames. The difference seems to amount to "type" versus "*.type".

I like that you can get all the files of a type without using REGEX.
Title: Re: OP2MapImager Development
Post by: Hooman on July 31, 2017, 02:38:41 AM
Quote
I could eliminate the directory parameter from both functions and assume ResourceManager only searches the directory that contains the pre-loaded archive files. I think it makes sense to be able to search other directories for files without having to create a new instance of ResourceManager.

Using the VOL search folder as the base for file searches would be closer to how the game works. It also simplifies use for the caller since they don't need to keep a reference to the install folder around and pass it for every call. If you want to load a file outside of that folder, you can always use a relative path from there. I think that would actually be the recommended way of doing it.

Avoid absolute paths. They're a huge pain if you ever move stuff around, transfer things between systems, or sometimes even just install something to a non-standard place.


Quote
I like that you can get all the files of a type without using REGEX.
It feels like you're reinventing the wheel.


Also, "#include <filesystem>" should be fairly cross platform. Probably not much need to guard most of XFile with "#if defined(_WIN32)".
Title: Re: OP2MapImager Development
Post by: Vagabond on August 01, 2017, 02:16:27 PM

Quote
Also, "#include <filesystem>" should be fairly cross platform. Probably not much need to guard most of XFile with "#if defined(_WIN32)".

When I chose filesystem, I was hoping it would allow for being cross platform. The problem is that it is still in draft form, so they may change the implementation whenever it is actually released. and I didn't know if compilers on other operating systems support it, so I still pushed all the filesystem specific code into XFile.

From https://docs.microsoft.com/en-us/cpp/standard-library/filesystem
Quote
As of the release of Visual Studio 2017, the <experimental/filesystem> header was not yet a C++ standard. Visual C++ 2017 implements the final draft standard, found in ISO/IEC JTC 1/SC 22/WG 21 N4100.

Hooman, which compiler do you typically use on Linux, and do you know if it has implemented this filesystem standard?

Quote
Using the VOL search folder as the base for file searches would be closer to how the game works. It also simplifies use for the caller since they don't need to keep a reference to the install folder around and pass it for every call. If you want to load a file outside of that folder, you can always use a relative path from there. I think that would actually be the recommended way of doing it.

Avoid absolute paths. They're a huge pain if you ever move stuff around, transfer things between systems, or sometimes even just install something to a non-standard place.

Currently the default folder is wherever OP2MapImager.exe is located. It does make more sense to switc this so ResourceManager uses the folder where you told it to find the archive files as the default directory.

I need a way to distinguish between the directory, relative or absolute, and the REGEX expression that the user wants to use to search for multiple filenames though right?

-Brett
Title: Re: OP2MapImager Development
Post by: Hooman on August 03, 2017, 02:50:18 AM
Quote
When I chose filesystem, I was hoping it would allow for being cross platform. The problem is that it is still in draft form, so they may change the implementation whenever it is actually released. and I didn't know if compilers on other operating systems support it, so I still pushed all the filesystem specific code into XFile.
Using XFile is fine. What I'm saying is that within XFile, you have a lot of "#if defined PLATFORM" style code. A lot of it appears to be preemptive, assuming there will be a problem before it even has a chance. As the calls go to <filesystem>, which itself should be cross platform, you should be able to just assume platform specific differences will have already been taken care of. If they're not, a compile error on that functionality is probably more useful than a generic error message about the platform not being supported. I'd suggest removing the the guards.

I use primarily g++ on Linux, and on occasion clang. I do have <experimental/filesystem> available.

Quote
Currently the default folder is wherever OP2MapImager.exe is located. It does make more sense to switc this so ResourceManager uses the folder where you told it to find the archive files as the default directory.

Yes, I think the ResourceManger should store the path it gets passed when finding the VOL and CLM files.

Quote
I need a way to distinguish between the directory, relative or absolute, and the REGEX expression that the user wants to use to search for multiple filenames though right?

You can convert relative paths (including regex/glob) to absolute by prepending a root folder. This might be the path to the VOL and CLM files. Paths are specified to the ResourceManager as relative to its own folder, which are made absolute before being passed to the OS. This avoids issues where changes to the current working directory cause unintended side effects.

You can avoid prepending if the path is already absolute. Though whether to accept such paths is another issue. In some cases accessing files outside of some root folder might be a security risk. In such cases, it may make sense to throw an exception rather than process the path.

Code: [Select]
	if (directoryStr == "" || directoryStr == "/" || directoryStr == "\\" || directoryStr == " ")
directory = current_path();
Here you might consider raising an exception for the "/" and "\\" cases. For the other two cases, the if is possibly meaningless, since the default handling for relative paths is to consider them relative to the current working directory (current_path()).

Also, it's concerning you're checking for " " (a single space). This strikes me as papering over a bug or bad design somewhere else in the code.
Title: Re: OP2MapImager Development
Post by: Vagabond on August 03, 2017, 06:07:41 AM
Quote
I use primarily g++ on Linux, and on occasion clang. I do have <experimental/filesystem> available.

Sounds good. I'll take the if Defined statements out then.

Quote
You can convert relative paths (including regex/glob) to absolute by prepending a root folder. This might be the path to the VOL and CLM files. Paths are specified to the ResourceManager as relative to its own folder, which are made absolute before being passed to the OS. This avoids issues where changes to the current working directory cause unintended side effects.

You can avoid prepending if the path is already absolute. Though whether to accept such paths is another issue. In some cases accessing files outside of some root folder might be a security risk. In such cases, it may make sense to throw an exception rather than process the path.

I want to ensure the directory does not include any REGEX and allow the filenames to include REGEX. So the user cannot search through multiple directories using REGEX, but they can search for multiple files in a directory. I'm not sure how to definitively separate out relative and or absolute directories from filenames that may include REGEX, so I think it is best to provide the directory and the REGEX defined filenames as separate variables.

I have no problems with the OP2MapImager searching anywhere on a user's computer that they specify for maps, archive files, and well files through either relative or absolute directories. I don't have any problem allowing them to save the renders anywhere they want. If this is a concern though, then I guess we could look into limiting to only saving files in My Documents or something?

Quote

Code: [Select]
	if (directoryStr == "" || directoryStr == "/" || directoryStr == "\\" || directoryStr == " ")
directory = current_path();
Here you might consider raising an exception for the "/" and "\\" cases. For the other two cases, the if is possibly meaningless, since the default handling for relative paths is to consider them relative to the current working directory (current_path()).

Also, it's concerning you're checking for " " (a single space). This strikes me as papering over a bug or bad design somewhere else in the code.

I need a way to receive from the user that they are attempting to search the current directory for maps. If they pass

Code: [Select]
OP2MapImager /
or
OP2MapImager \

it will image all maps in the current directory. I'm not really experienced with the command prompt. If there is a better way to handle, I'd be happy to know about it.

I did just check, and you cannot pass an empty string through a command prompt argument, so an empty string or a single space aren't really an option. I wasn't sure when first writing so I threw them both in.

All of XFile is pretty unpolished right now. I was learning how to use the filesystem library code when writing it and didn't spend a lot of time cleaning it up. Since filesystem should work on both Linux and Windows, and you seem comfortable using it even in draft form, it would be good to spend some time cleaning it up.



Update. I pushed some changes to the code base to try and do some cleaning.

I want to simplify the function getFilesFromDirectory. However, if directory_iterator is used with a path that is empty (""), then it will not iterate over the current directory. I have to set the directory via current_path in order to make it work. That is causing the weird check the path against the directory in the first 3 lines.

Does it make sense to allow iterating over the directory if a path is passed into the function that contains a filename? If no, then I could simplify by just throwing an error if a filename exists at the end of the path.

I can move the line to check if the path string is a forward/backwards slash to the mapImager main.cpp file. Then it would be an implementation detail for how the user tells OP2MapImager how to iterate over the current directory. This might make more sense than placing it in XFile. This would apply to both versions of getFilesFromDirectory.

Code: [Select]
vector<string> XFile::getFilesFromDirectory(const string& pathStr, const string& fileType)
{
path p(pathStr);
path directory(p.remove_filename());
if (p == directory)
directory = current_path();

if (pathStr == "/" || pathStr == "\\")
directory = current_path();

vector<string> filenames;

for (auto& directoryEntry : directory_iterator(directory))
{
if (directoryEntry.path().extension() == fileType )
filenames.push_back(directoryEntry.path().string());
}

return filenames;
}
Title: Re: OP2MapImager Development
Post by: Hooman on August 03, 2017, 04:36:54 PM
Quote
I want to ensure the directory does not include any REGEX and allow the filenames to include REGEX. So the user cannot search through multiple directories using REGEX, but they can search for multiple files in a directory.
Why? I don't see a reason for this restriction.

Quote
I'm not sure how to definitively separate out relative and or absolute directories from filenames that may include REGEX, so I think it is best to provide the directory and the REGEX defined filenames as separate variables.
Absolute/relative is an orthogonal concern to fixed/glob/regex.

A relative path is not complete. It can not be used to access files or folders. Either you must prepend a root path, or the OS will prepend the current working directory as the root path, and from there it will match using the absolute path.

You need a fixed path to open a file or folder. If instead you have a glob/regex, it must be used to search for a match. Any matches then become the fixed path to operate on, usually one at a time in a loop.

It makes perfect sense to use a glob or regex or match multiple folders, even if the filename portion ends up being fixed. Ex: "**/makefile" to find any "makefile" in any of the subfolders rooted at the current working directory. The "**" part will match zero or more folders. It's not found in every environment. It's common in the Ruby and Javascript worlds, but doesn't work in the command shell.

Quote
I have no problems with the OP2MapImager searching anywhere on a user's computer that they specify for maps, archive files, and well files through either relative or absolute directories. I don't have any problem allowing them to save the renders anywhere they want. If this is a concern though, then I guess we could look into limiting to only saving files in My Documents or something?

Don't worry about it for your map imager, but it may be a concern for a more general utility library. If such a component is used in a game that support multiplayer, and the other computers can influence which files are accessed, that can be a problem. Consider a game that support resource downloading, such as map files. Maybe it takes a map filename such as "custom.map", or "campaign/01.map", which by default will be in the game's "map/" folder. If instead, the path is "/etc/passwd", or (some arbitrary number of) "../../etc/passwd", or "symLinkToRoot/etc/passwd", then you could potentially trick the client into uploading its (Linux) password file. A contrived example here, since the host distributing the map is usually the one choosing the map. You could extend the idea though to the resources accessed by the map. Maybe the map references "/etc/passwd", so when you send the map to other players, their computers may try to access "/etc/passwd", which might get loaded into the game and somehow leaked back to the host.

You can defeat the absolute path attack by unconditionally adding a non-empty prefix. You can defeat the ".." attack by expanding the path to a canonical form, and ensuring the prefix still matches and there are no ".." left (or hack it, and just disallow any ".."). You might also want to turn off following symlinks in case a name links to a file or folder in a parent directory. This is sometimes handled by expanding to a canonical path, which may translate symlinks to their targets.

Quote
I need a way to receive from the user that they are attempting to search the current directory for maps.

You could check the count of the number of arguments, and if absent set a default path of "" for the current folder. Failing that, the user could specify:
Code: [Select]
OP2MapImager ""
Or even:
Code: [Select]
OP2MapImager ./

Quote
I want to simplify the function getFilesFromDirectory. However, if directory_iterator is used with a path that is empty (""), then it will not iterate over the current directory.
That is indeed unfortunate, and very unexpected. This may warrant further investigation.

Code: [Select]
if (pathStr == "/" || pathStr == "\\")
directory = current_path();
This is very unusual. Such a path means absolute, not relative, and so it's unexpected that it would end up being based off the current working directory. I may have to take a closer look at this code.

Have you exported a makefile for the relevant projects?
Title: Re: OP2MapImager Development
Post by: Vagabond on August 04, 2017, 04:59:04 PM
Code: [Select]
OP2MapImager ./

I played more with filesystem. filesystem::is_directory returns false upon passing a blank string (""). So, based on this, I'm thinking that filesystem is purposely not iterating over the current directory when a blank string is provided as the path. When I use a dot (.) or ./, everything works properly though. Reading the specification document for filesystem would possibly answer the question definitively, but I'm happy just using ./.

I had forgotten that ./ is used for the current relative directory. :-[ I reworked the OP2MapImager to take either . or ./ for indicating the current directory. Not allowing "" for a relative directory will keep someone from iterating over 100 maps by accident just because they left off the filename by accident. It also plays nicer with filesystem.

It didn't occur to me you could actually pass a blank string or a single space as an argument by using quotes. Experimenting with it a little teased out a bug when you pass in an empty string. I've fixed this bug on the development version.

Some point in the near future I'll release a new version of the OP2MapImager with both of these issues fixed.

So, that simplifies getFilesFromDirectory to the code below (minus the 3 commented out lines in the middle)

Code: [Select]
// For current relative directory use . or ./. An empty string is not considered a valid directory.
vector<string> XFile::getFilesFromDirectory(const string& directoryStr, const string& fileType)
{
path directory(path(directoryStr).remove_filename());

// Brett208 4Aug17: creating a path with an empty string will prevent the directory_iterator from finding any files in the current relative path on MSVC.
//if (directoryStr == "")
// directory = path(".");

vector<string> filenames;

for (auto& directoryEntry : directory_iterator(directory))
{
if (directoryEntry.path().extension() == fileType )
filenames.push_back(directoryEntry.path().string());
}

return filenames;
}

Quote
Have you exported a makefile for the relevant projects?

No, but we will need to decide how to build the project on Linux at some point. I was considering just creating a completely new solution/project in whatever IDE you prefer on Linux and manually configuring required properties. We will have to load in the FreeImage.dll designed for Linux and create a separate post build event that complies with whatever IDE you want to use since I'm doubting it accepts Windows batch scripts. I've read up enough to know sort of what a makefile is and does, but not enough to use in practice. However it is done, I'm sure to learn a lot in the process.

Oh course first the Windows.h header needs to be teased out of the Archive code.

I was thinking to create a Console Application that finished defining the required functions to manipulate archive files first. This way we can test the results of changes to the archive code as windows.h is removed. Then perhaps circle around to the Linux build of everything.
Title: Re: OP2MapImager Development
Post by: Hooman on August 05, 2017, 06:16:38 PM
MSVC has an option to export a Makefile. It might be easier to use that than generate a new one by hand. Though the NAS2D makefile is pretty generic, so maybe we could just reuse that.


Teasing out the "#include <windows.h>" will be a bit of a pain. It might be easier if we used regular stream code rather than memory mapped files. Perhaps converting that should be the first step.


I played around with the <experimental/filesystem> code today on Linux. Indeed, the directory_iterator does not like empty path strings. It also doesn't like shell globs, and it doesn't like filenames either. It must be a non-empty folder name.


The filesystem library does handle converting paths between various path/string/char* formats. That could allow some slight simplification to the code. For instance, directory_iterator can take any of those 3 types, which means you don't need to convert a string to a path before calling it. You also don't need to explicitly convert a path to a string before adding it to the vector.

After playing around a bit, I built out my example to be similar to your code, minus the filename extension checks. Here's what it looked like:
Code: [Select]
std::vector<std::string> getFilesFromDirectory(const std::string& dir)
{
  auto pathStr = dir.length() > 0 ? dir : "./";

  std::vector<std::string> filenames;
  for (auto& entry : fs::directory_iterator(pathStr))
  {
    filenames.push_back(entry.path());
  }

  return filenames;
}


Oh, and I'd say just assume the folder name passed is an actual folder, and not a file. Trying to strip a filename from the path and use the containing folder is a bit unexpected and could lead to subtle bugs in library use. I'd say just let the path feed through to the directory_iterator, where it will cause an exception if it refers to a file rather than a folder.
Title: Re: OP2MapImager Development
Post by: leeor_net on August 05, 2017, 07:09:19 PM
Would it make sense to use something like PhysicsFS to handle the cross-platform file handling? I've used it with great success.

Though it may make more sense to grab the code from TrenchBroom (map editor). It has an abstract filesystem that seems to have everything for Windows/Linux/Mac and it works pretty well once you get the hang of working with it:

https://github.com/kduske/TrenchBroom/blob/master/common/src/IO/FileSystem.h
https://github.com/kduske/TrenchBroom/blob/master/common/src/IO/FileSystem.cpp
Title: Re: OP2MapImager Development
Post by: Hooman on August 05, 2017, 08:02:44 PM
Upon cursory inspection, it looks like a nice API. Though I tend to shy away from adding external dependencies when they can be avoided. It complicates distribution and compiling.

I suppose that's one of the nice things about the Ruby world, or NodeJS. They have their own language specific packaging and distribution system, and the language core libraries insulate most code from platform considerations, so source is often highly portable between systems. It makes it very easy to incorporate external code, without adding significant difficulty for distribution, compiling, or platform considerations.
Title: Re: OP2MapImager Development
Post by: Vagabond on August 06, 2017, 03:52:26 AM
leeor_net,

It looks like a clean library. The goal of using filesystem is that it may eventually become part of the standard library, which means many people would become familiar with it. Of course, since it is experimental right now, maybe using it isn't worth the squeeze.



Hooman,

Are you using an alias of fs for the namespace std::experimental::filesystem? If not, it looks like they named the namespace differently between the two compilers.

Code: [Select]
for (auto& entry : fs::directory_iterator(pathStr))
Produces

Error (active)   E0276   name followed by '::' must be a class or namespace name   



I'm getting E0304 from the line of code below. If I change it to entry.path().string(), then it compiles. So it looks like there are some differences between the two compilers implementation.

Code: [Select]
filenames.push_back(entry.path());
Error (active)   E0304   no instance of overloaded function "std::vector<_Ty, _Alloc>::push_back [with _Ty=std::string, _Alloc=std::allocator<std::string>]" matches the argument list

Here is the function rewritten to work in VS17. Since I have a using statement, declaring experimental::filesystem namespace for directory_iterator isn't necessary, but left it to point out the difference.

Code: [Select]
std::vector<std::string> getFilesFromDirectory(const std::string& dir)
{
auto pathStr = dir.length() > 0 ? dir : "./";

std::vector<std::string> filenames;
for (auto& entry : experimental::filesystem::directory_iterator(pathStr))
{
filenames.push_back(entry.path().string());
}

return filenames;
}

Also, if we want to accept empty strings as meaning the same thing as ./ or ., we should update the following two functions to read this way.

Code: [Select]
bool XFile::isDirectory(const string& pathStr)
{
if (pathStr.length() == 0)
return true;

return is_directory(pathStr);
}

Code: [Select]
bool XFile::pathsAreEqual(string pathStr1, string pathStr2)
{
if (pathStr1.length() == 0)
pathStr1 = "./";

if (pathStr2.length() == 0)
pathStr2 = "./";

path p1(pathStr1);
path p2(pathStr2);

return p1 == p2;
}

If this all looks good, I'll commit the changes.

-Brett
Title: Re: OP2MapImager Development
Post by: Hooman on August 06, 2017, 09:51:33 AM
Ahh yes, I forgot the namespace line:
Code: [Select]
#include <experimental/filesystem>
namespace fs = std::experimental::filesystem;


The blank path was a problem for directory_iterator. I don't know if it's an issue for is_directory or path object comparisons. I had no trouble setting a path object to the empty string. I only had problems when I tried to use an empty path object with directory_iterator.

Maybe check what happens when is_directory is passed an empty string. As for comparing path objects, I'm wondering how robust the comparison is. A simple string compare won't be very effective in some cases, though I don't know if it does a string compare or something more involved. What if you compare "." and "./"? What if you compare "./" and "./subFolder/../"? In some cases it may make sense to do a simple string compare, to see if two paths are the same and in the same form. In other cases, you might want to know if two paths refer to the same thing, even if they are given in different forms.
Title: Re: OP2MapImager Development
Post by: Vagabond on August 06, 2017, 06:18:00 PM
Just committed changes to the repo.

There are now 3 versions of getFilesFromDirectory. The one Hooman just wrote, one using regex for the filename, and one that uses the file extension. The regex and file extension overloads first call the generic getFilesFromDirectory and then remove files that do not meet the regex or file extension requirement. This way we do not have to repeat the line of code forcing a blank string into "./" and the directory iterator in each function.

Code: [Select]
vector<string> XFile::getFilesFromDirectory(const string& directory, const regex& filenameRegex)
{
vector<string> filenames = getFilesFromDirectory(directory);

for (int i = filenames.size() - 1; i >= 0; i--)
{
if (!regex_search(filenames[i], filenameRegex))
filenames.erase(filenames.begin() + i);
}

return filenames;
}



I have already tested is_directory with a blank string. It will return false, which makes sense in light of the directory_iterator not working with a blank string.



I played some with comparing paths. Using == against filesystem::path basically compares if the underlying string is the same. So == will return "." and "./" as different (false). However, the filesystem::equivalent function will return "." and "./" as being the same (true).

So, I rewrote the function as pathsAreEquivalent. The areas I was using pathsAreEqual in the code base, I was using it to compare if the filename itself was the same. Now in the code base, I just changed it to compare (==) the raw strings or used strcmp in the archive code since it uses char* instead of std::string.

Code: [Select]
// Will throw an exception if passed only a filename as path1 or path2. Lead filenames with relative or full directory.
bool XFile::pathsAreEquivalent(string path1, string path2)
{
if (path1.length() == 0)
path1 = "./";

if (path2.length() == 0)
path2 = "./";

return equivalent(path(path1), path(path2));
}

Test Data:

Code: [Select]
string d1 = ".";
string d2 = "./";
string d3 = "./Debug/../";
string d4 = "";

bool equal = XFile::pathsAreEquivalent(d1, d2); // TRUE

equal = XFile::pathsAreEquivalent(d1, d3); // TRUE

equal = XFile::pathsAreEquivalent(d2, d3); // TRUE

equal = XFile::pathsAreEquivalent(d1, d4); // TRUE
Title: Re: OP2MapImager Development
Post by: Vagabond on August 07, 2017, 05:03:08 AM
Just committed code again. Fixed bugs when creating pathsAreEquivalent function. I got bit by trying to use == against 2 char* in the archive code since it uses char* instead of string. Ran out of time to fix when working last time so it was in a bit of an awkard state. Everything is working now.



We need to consider how to handle filenames being the same on different operating systems. For example on Windows tutorial.map is the same as Tutorial.map, but they are different valid filenames on Linux I think? So, simply comparing 2 filenames using == or strcmp may not be appropriate.

I think we want manipulated files to remain compatible on both systems. If on Linux you create tutorial.map and Tutorial.map and then pack them both into maps.vol, Windows will not be able to tell them apart. So really, the code should use the more restrictive option and treat tutorial.map and Tutorial.map as the same to prevent stuff like this from happening. It may not be initially intuitive to a Linux user at first, but we can comment the code to indicate the differences.

I've removed parthsAreEqual from XFile. In retrospect, do we want to re-add the function and use it to force Linux to consider ./tutorial.map as an equal path to ./Tutorial.map?

-Brett
Title: Re: OP2MapImager Development
Post by: Hooman on August 08, 2017, 05:47:48 AM
Quote
There are now 3 versions of getFilesFromDirectory. The one Hooman just wrote, one using regex for the filename, and one that uses the file extension. The regex and file extension overloads first call the generic getFilesFromDirectory and then remove files that do not meet the regex or file extension requirement. This way we do not have to repeat the line of code forcing a blank string into "./" and the directory iterator in each function.

Ahh, I wasn't expecting you to snaffle my function. That's fine. Sounds like you've simplified the code. Though I should point out it could be considerably less efficient to build the full file list, and then filter it to remove items you don't want. That is especially true of a large folder with only a few files of interest.

Seems like a template function could be written that takes some sort of filter function. That could eliminate code duplication, runtime costs of building a list and then filtering, and also dispatch cost for the filter function. A default filter function could be set that doesn't remove any filenames. Not quite sure how it would work though.

Code: [Select]
return equivalent(path(path1), path(path2));
What is "equivalent"?

I like how you have test code. Is it wrapped in a test suite? I was thinking having a few unit tests could be good for some of these projects.


The case sensitivity thing is more an issue of the file system than the OS. Think how Linux accesses NTFS or FAT partitions, or what happens when Windows accesses a file over a network share that's hosted on Linux. Sometimes there is a bit of support to paper over differences, sometimes not.

The VOL files are essentially their own file system. Whether they are case sensitive or not is up to the design of VOL files. I suppose I could check the Outpost 2 code to see if it uses case insensitive comparisons. Mostly though I'd suggest not bothering. It's not likely to be an issue in practice, and if it is, the user will probably be the best person to handle it.
Title: Re: OP2MapImager Development
Post by: Vagabond on August 09, 2017, 04:48:02 AM
Equivalent means the two supplied paths return the same underlying directory or filename.

I can do generics in C#, but templates in C++ are beyond my current skill without some more studying/research. I'm not opposed to learning it though. I agree that it could be inefficient, but it also isn't noticeably affecting the execution speed of any of the consuming code.

I'll try not to get too wrapped up in case insensitivity then.

For testing the equivalent function, I just stuffed some code in the main function and stepped through it in the debugger to record the BOOL results. I'm not opposed to setting up unit tests, but I think typically they are done through some sort of framework. I have only done a little unit testing and that was all through C#. I guess we would need to discuss the strategy we wanted to use. I like that D has unit test built into the core of the language specification.
Title: Re: OP2MapImager Development
Post by: Hooman on August 09, 2017, 06:37:34 AM
Quote
Equivalent means the two supplied paths return the same underlying directory or filename.

That feels vague. How is it determined? If "a" is a symlink to "b", does 'equivalent(path("/path/to/a"), path("/path/to/b"))' return true?

You're right about the inefficiency not affecting the program in question. There are bigger fish to fry.

I've never really done much unit testing in C++, at least not what I would call proper unit testing. Some cursory searching reveals there is a Google Test library that seems to be quite popular. It might be good to start developing a culture of unit testing here. Seems recent projects are taking off, and more people are starting to collaborate. Unit testing would be good for that.

Thought to bring this all back from things we could, or maybe should do, what are the specific objectives for the project. Maybe some of this is a bit of a distraction.

It seems the OP2MapImager is a basically working and complete project. Do you want to add more features, continue polishing it, or move on to something else? All are valid options.
Title: Re: OP2MapImager Development
Post by: Vagabond on August 10, 2017, 04:20:22 AM
Quote
That feels vague. How is it determined? If "a" is a symlink to "b", does 'equivalent(path("/path/to/a"), path("/path/to/b"))' return true?

I deleted the equivalent function and replaced it with the pathsAreEqual function. I was getting frustrated with the filesystem::equivalent function because it throws errors if the path doesn't actually exist on the hard drive. pathsAreEqual ignores case and treats "./tutorial.map" and "tutorial.map" as equal. So now if you type "./Tutorial.map" it will recognize that you want to render "tutorial.map". It also won't throw an error if you are comparing filenames that only exist in an archive files. I didn't really have any use cases for pathsAreEquivalent anyways.

Quote
Thought to bring this all back from things we could, or maybe should do, what are the specific objectives for the project. Maybe some of this is a bit of a distraction.

Besides re-releasing the map maker with these bug fixes, I just want to get the OP2Utility code to a point where you are happy with its state when we are ready to port it to Linux. OP2Utility is also being used by OP2Archive. Otherwise, I'm finished with the application until someone has a problem with it.

If we are serious about unit testing than we can either use this project as a test case or start by hooking it into OP2Archive (sorry for the pun :) ). I'll check out Google Test in the near future. If it is popular, then it is probably a good choice because it should be supported for a long time, people may already be familiar with it, and learning it may help directly apply to projects outside of Outpost 2.

Leeor,

I don't suppose OutpostHD/NAS2D uses any unit test library???

Also, I'll be uploading my changes shortly to the REPO. I kind of fell behind on this.

-Brett
Title: Re: OP2MapImager Development
Post by: leeor_net on August 10, 2017, 01:43:16 PM
I don't suppose OutpostHD/NAS2D uses any unit test library???

No -- I never got around to it. What I've been doing instead is building small programs that test core functions/classes/objects and confirming output/reliability either visually (with the renderer test, test2 in NAS2D-Tests) or with text output.

NAS2D's test programs are here: https://github.com/lairworks/nas2d-tests

The Renderer test (test2) is the most complete test though there are other functions I haven't added to it yet to inspect their output.

For OutpostHD, I built small sample programs that test specific parts of the simulation. There are three at the moment, the first and second revisions of the Population model and the PopulationPool object to ensure correct behavior: http://redmine.outpostuniverse.org/projects/outposthd/files

I don't know if these could be considered 'unit tests' or if they'd even be helpful. But, there they are. :)
Title: Re: OP2MapImager Development
Post by: Hooman on August 12, 2017, 02:55:52 AM
Quote
I don't suppose OutpostHD/NAS2D uses any unit test library?

I was thinking similarly.

Leeor_net, nice to see those tests. I didn't even notice they existed before. I took a quick peek. They're not quite unit tests, in that results are presented in a MsgBox. Usually unit tests have an automated pass/fail criteria, and console logging of the test results. This makes it easy to have them run automatically, often after every commit, or pull request, and the results logged to be inspected later. Inspecting the output would be similar to the Travis CI or Codacy output setup for the GitHub repo.

In fact, such tests could be run by the Travis CI build. Though, usually unit tests get added to the same repo, which makes the process a little easier. Makes sense though, since unit tests are usually added and updated along with the code they are testing. You can keep the tests out of the "src" folder though, perhaps in a separate "test" folder. The build system doesn't need to build the test code by default.
Title: Re: OP2MapImager Development
Post by: Vagabond on August 12, 2017, 04:51:01 PM
I spent some time reading through Google Test's documentation.

The documentation seems clear to read. It seems like robust support for both Windows and Linux, although based on some of the documentation, I tend to think it wasn't developed for Visual Studio primary (which is fine). I liked how they pointed out in their documentation where they differed some from a standards document on unit testing and that the standards were in fact stated better than what they had developed. Shows a healthy respect for standardization.

It also seems pretty in line with the ideas/theories that I've used in my limited C# based unit testing in the past.

I'll look into downloading the Linux and Windows builds and see how big the downloads are, etc. I'd probably prefer pushing this into OP2Archive first over back-fitting OP2MapImager though.

Happy Coding!

-Brett