search

OutpostUniverse / OP2Archive

Console Application for examining, creating, and extracting files from Outpost 2 .vol and .clm archives.
https://wiki.outpost2.net/doku.php?id=outpost_2:helper_programs:op2archive
MIT License
1 stars 0 forks source link

Reduce char star usage #10

Closed Brett208 closed 6 years ago

Brett208 commented 6 years ago

Remove unnecessary calls to string::c_str

Update struct ConsoleSwitch to use constructor initialization list

Brett208 commented 6 years ago

The update to the submodule of OP2Utility is listed as follows from TortoiseGit:

Revision: f42ae5a38c0a24ea164db12deb78ee0021adcef9 Subject: Merge pull request #100 from OutpostUniverse/Standardize-loops

In the log, this commit has a pink highlight saying 'super-project-pointer', whatever that means.

-Brett

DanRStevens commented 6 years ago

FYI upon checking out this branch, I get a compile error since the update to the ClmFile constructor to take a std::string is not seen.

I've mucked about a bit, but currently running "git status" shows:

modified:   OP2Utility (new commits)
Brett208 commented 6 years ago

Now that OP2Utility's master branch contains the associated changes, I updated this branch to use the master of OP2Utility. Can you see if that fixes your compile time error?

DanRStevens commented 6 years ago

I was getting status messages about an updated submodule. I had to play around with manually updating the submodule and switching branches. I'm not sure if that's because I didn't use the right set of flags, or because I'd played with things earlier and put it in a non-default state. Regardless, it did compile in the end.

Brett208 commented 6 years ago

This ConsoleArgumentParser is currently duplicated in OP2MapImager.

@DanRStevens, do you have any thoughts on a ConsoleArgumentParser? We could move this code into OP2Utility as a separate folder for both projects to reference. However, I'm not sure the code is generic enough for other projects to benefit from, it isn't well refined, and it isn't really specific to OP2. Then again I guess streams are not specific to OP2 either.

I'd also be content switching it out for some sort of existing lightweight parser that was well used by others if it has the right license.

I'm not planning to play with console parser code much more until some sort of decision is made on what to do with it. It seems stable though so doesn't really need work in the near future.

Thanks, Brett

DanRStevens commented 6 years ago

It does seem generic enough to be in a library. Or to have used a library for it. I haven't reviewed it much though, so we might want to take a look at the design. You're right that it's not specific to OP2, and neither is the Streams library. Perhaps we should consider breaking that out into its own library at some point? Seems convenient for now to have it all in one place.

I suppose we can just leave it for now. There are bigger fish to fry. Maybe review the design and placement at a later point.