[5802] compile problem

[Dilvish]

ok, well BigJoe, with that last patch you requested we test, applied to version 5802, I first wound up with a few "need typname" kind of errors that were straightforward to rectify; on the second compile attempt it came up with some other errors of a similar kind to what have been popping up with previous versions; one of them was the 'can't call member function without object' problem. I've attached the build log.

[Bigjoe5]

my patch program says it's stripping extra CRs from the ends of lines for these patches, but it still rejects matching of all chunks (didn't used to see this problem with patches I downloaded here). Anyway, I'll do it manually & report back in a few minutes

Oh, yeah, that's my bad. Visual Studio gave me an option to normalize all the EOLs for the file, since apparently some of them didn't match, and I just accepted its offer without thinking.

ok, well BigJoe, with that last patch you requested we test, applied to version 5802, I first wound up with a few "need typname" kind of errors that were straightforward to rectify; on the second compile attempt it came up with some other errors of a similar kind to what have been popping up with previous versions; one of them was the 'can't call member function without object' problem. I've attached the build log.

That also seems like something VS should have caught, but didn't... I suspect just replacing the references to map::const_iterator member functions from classes which now derive from map::iterator would fix that specific problem.

[Vezzra]

Vezzra and Dilvish, would you mind testing that change before I commit it? I'll post a patch here when it's done.

Sorry for my somewhat infrequent responses, but I'm confronted with some difficult situations I need to deal with in my private life which are pretty time-consuming right now (and will stay so for a while). If you're going to put out another patch, of course I'll try to find the time to test it, but because of this situation it may take me longer than usual.

Aside from that, I tried to build FO with the fixes you committed (just to make sure), everything worked.

[Bigjoe5]

Here's a patch that hopefully gets rid of the need for Dilvish's odd workaround by providing a constructor for the const_value_iterator taking a single argument of type iterator<T>. This should allow the result of calling begin_values on a non-const UniverseObject to be converted to a const_iterator. If this doesn't work, we may need to just have different names for the const functions.

This patch also has everything being passed into the constructors by const reference, just to make sure everything that's been suspected to cause compilation errors is out of the way with this patch.

[Vezzra]

I applied your patch to rev 5814, build failed on OSX. Complete buildlog attached.

[Bigjoe5]

Thanks for testing. It looks like the only problems are of this nature:

Code: Select all

/Users/user/SoftwareProjekte/FO/current/Xcode/../UI/SidePanel.cpp:2300: error: conversion from 'ObjectMap::value_iterator<System>' to non-scalar type 'ObjectMap::const_value_iterator<System>' requested

I guess I will have to use different names for those functions after all, so we can get a const_value_iterator from a non-const ObjectMap. I'll prepare another patch later tonight and report back.

[Dilvish]

I got the same three errors that Vezzra did. These were similar to what we saw last time, though & I was able to get it fixed & run it fine (just tried a few turns).

The basic problem appears to me that the const_value_iterator constructors both require a const iterator to be fed to it and "Objects().begin_values<System>()" was not doing that. Even the constructor that appears at first glance to apply to any iterator -- the one for "iterator<T> base" -- when you follow up on that iterator<T> it looks like, at least for T=UniverseObject, thats a const iterator type. So there was no const_value_iterator constructor accepting a non const iterator. I tried following up on ObjectMap::begin_values to see why it wasn't coughing up a const iterator, but the definition for const ObjectMap::begin_values led me to the definition for plain ObjectMap::begin_values, which then only led my IDE to the declaration, without me ever seeing that was really being provided & I didn't chase it down further.

The better solution is probably to provide a suitable constructor for const_values_iterator; my solution was to go to those three problem loops and wrap the references to Objects().begin_values<System>() : std::map<int, System*>::const_iterator(Objects().begin_values<System>()) which was something the const_values_iterator constructor could already accept. I did the same for those thee calls to Objects().end(), but first changing it to Objects().end_values() (without checking if that was even necessary)

Attached is a revised patch relative to 5814 -- Vezzra, care to give this a try?

[Bigjoe5]

There are about 100 places in the code I'm planning on using this iterator... I'd rather not have to wrap it in a verbose constructor every time.

Here's a patch that changes the name of the function that returns a const_value_iterator so it can be called on a non-const ObjectMap.

[Dilvish]

Here's a patch ...

ok that worked. The patch was a bit odd, in that 3 out of 5 of the ObjectMap.cpp hunks failed to apply, but they weren't bad to add manually, and then it compiled and ran fine.

[Bigjoe5]

Here's a patch ...

ok that worked.

Committed.

The patch was a bit odd, in that 3 out of 5 of the ObjectMap.cpp hunks failed to apply, but they weren't bad to add manually, and then it compiled and ran fine.

I think there are some weird line ending issues, that aren't being helped by the fact that I don't know what line-ending format we use as the standard for FO files...

edit: Hopefully it wasn't premature to commit this before Vezzra tested it... it seems unlikely that there would be a problem, and I wanted to get it out of the way before the upcoming week.

[Geoff the Medio]

I don't know what line-ending format we use as the standard for FO files...

For C++ files, there isn't one.

I always attempt to convert to Unix linefeed only format when given the option, though it only really comes up when I've been editing a file and MSVC randomly decides to ignore the current file default and use whatever it feels like for some of the edited lines (ie. DOS style in an otherwise Unix style file), leaving the file inconsistent. Closing and reopening a file in the MSVC editor will pop up a dialog if the loading file has inconsistent line endings, and it will auto-fix them if so instructed. It's difficult to remember to do this every time, though, when can cause problems if making a patch.

I think SVN also does some stuff automatically when committing and updating / checking out to make things automatically work on whatever system you're using... So if I'm right, a file can appear to be Unix style on a Linux system and DOS style on a Windows system and SVN is fine with that. However, commit doesn't like it if given inconsistent line endings when trying to commit, often requiring the aforementioned reopening in the MSVC editor to fix.

I think the patching problems mostly arise when doing a diff on one system and trying to use it on another with different line ending styles; the diff and patcher seem to not handle changing or mixed or different styles as well as the update and commit systems.

[Dilvish]

I think the patching problems mostly arise when doing a diff on one system and trying to use it on another with different line ending styles; the diff and patcher seem to not handle changing or mixed or different styles as well as the update and commit systems.

Yeah, ok, it does look like that is the problem. At least now I know an easier than manual patching if it fails again -- convert all the CRLFs to LF only in the base file before patching -- if that's ok? (sounds like it is)

I had thought my patch program could handle the CR/CRLF issue ok because it recognizes CRs at the end of a line in the patch file, and for most of these patches (including the one that just gave me trouble) it reports that is stripping CRs. However, it looks like it is not so flexible in considering the target file -- The hunks that worked corresponded to those sections of the file ending in LF only (except the first hunk had one context line ending in CRLF on the base file and was reported to pass with fuzz 1).

[Vezzra]

edit: Hopefully it wasn't premature to commit this before Vezzra tested it... it seems unlikely that there would be a problem, and I wanted to get it out of the way before the upcoming week.

Actually, I did run into a couple of problems, but they weren't related to your patch. First I had to battle my own stupidity, then I ran into build errors caused by the new ModeratorAction classes serialization code. Peeking into the other Serialize....cpp's gave me an idea how to fix it, so I was finally able to build the head revision.

So, as far as OSX is concerned, your latest fixes to the ObjectMap iterators work.