member variable prefix Thread

[lgromanowski]

For reference:

Spoiler: Show

Okay. First the repository: I just created a new branch (next) this will serve as base and as integration end-point for 0.11.0 while 0.10.0 is still WIP (next kinda takes the role of master for now from a 0.11.0 point of view).
You need to create a new branch from your master branch (call it mprefix or something) and then merge in my next branch. Don't forget the submodule update!

Now you need to go through all struct and class definitions and look for member variables, that don't follow the mSomeName scheme, .e.g. when you have "name", it should be replaced by "mName".
You can probably leave alone dumb structs with only public member variables and no functions (even if the member variables there don't have an m-prefix, it hardly matters).

Then compile! Your compiler should provide a large number of error messages, indicating where these variables are used. Fix each one by modifying the name accordingly. Repeat as often as needed.

I suggest you leave libs/openengine and libs/mangle alone for now. These involve submodule handling and require special treatment.

Also, I suggest not to use search and replace or any other kind of automated tool. These are very likely to mess up your code. Letting the compiler find the places requiring change is safer.

When you are done, commit and push to your mprefix branch on github (which will automatically create this branch in the github repository).

PREPARATION:
Created new branch mprefixtask and checkouted to it.
Github doesn't show yet. Really wish they'd put this management stuff on the site, too.
git pull zinnschlag next // Looking good.
git submodule update //some updates...interesting. Didn't do this when I updated and pulled on my main branch...
git merge zinnschlag next //up to date.

Okay. That wasn't so bad. I'll see what I can start soon. Back to making random numbers and queueueueues.

Github doesn't show yet

Github can't show it, because they don't have it. The repository on github is not the same you have on your HD.

When you pull via pull (instead of fetch) git automatically does a merge. Therefore the last merge was a no-op.

You mixed up the order of merge and submodule update. Didn't cause trouble only, because the pull provided an implicit merge, which made the order right again.

Ah, I see. I'll have to remember to do the submodule update first. That'll go in the flowchart.

I'm good to go, though, right? Since merge is implicit with pull, it should be in order? (just making the merge redundant?)

Should I do anything to fix it?

No fixing needed. All good. My lack of precision when describing the task and your mistake cancel each other out.

I didn't forget about this. Will start in about a week when midterms clear up.

Okay. Please merge in my master thread, before you work on the code. There have been some changes, that would make a later merge more complicated (I believe).

Edit: And don't forget to do a submodule update.

Finally got around to write the naming conventions page: http://openmw.org/wiki/index.php?title= ... onventions

Please review the Member Variable section! There is a slight enhancement over the original naming scheme for this task.

No problem. Will do come time.

We agreed to not change member variable names in dumb structs, right? Pretty much everything in components/esm falls in this category.

I am bringing this up, because if you change components/esm (without merging in my master again) we could end up with a pretty nasty merge.

We agreed to not change member variable names in dumb structs, right? Pretty much everything in components/esm falls in this category.

I am bringing this up, because if you change components/esm (without merging in my master again) we could end up with a pretty nasty merge.

I don't think we discussed it - but from memory, all we had thus far was going to cover only files in apps/openMW/ for now.

It would not make sense if we decided not to include the data classes (dumb structs = abstract data type?) or library/component implementation stuff in all that until absolutely necessary.

I don't think we discussed it - but from memory, all we had thus far was going to cover only files in apps/openMW/ for now.

I think in the mission statement I somewhere mentioned, that changing dumb structs was optional.

dumb structs = abstract data type

Nope. Quite the opposite. Dumb struct = struct that consists mainly of a collection of public data members and does not provide any abstraction.

Huh. Goes against what I'm being taught right now. I wonder what it's for?

No problem, though - if it has to be left alone, then it'll be so. Or, if you want, I can put that subtask at the end.

Well, it doesn't have to be left alone, but it is probably a lot of work and has little benefit.

Huh. Goes against what I'm being taught right now. I wonder what it's for?

Its not uncommon to group a couple of variables into a struct to hand them around together. The structs in components/esm are indeed suboptimal though. It is a basic design flaw, that happened in the very early stage of OpenMW. Nico insisted for a while in keeping it and when he finally agreed that it was a bad idea, it was too late to change it (would basically mean to start from scratch, since this is a very central component).
Gave me quite a headache for a while, but we have workarounds in place now.

I really wish making and building on windows wasn't such a hassle.

I was going to start today after updating and pulling, but I thought It'd be a good idea to stick to MSVC, and now I can't make OpenMW again since we added bullet. I managed to make and build bullet but I still can't get cmake to be happy for the include directory, for which there isn't one, and nothing I've read is working so far.

I'm two minutes from doing this on Notepad, but I'll just leave it alone until I can actually make a project.

wiki dev setup still needs a serious update.

I had the same problems with building on windows after we integrated bullet, check the thread I made in support. Also make sure you set a BULLET_ROOT variable in cmake.

EDIT: Nevermind, saw your post in my thread. I hope you figure it out, and I will try to make suggestions if I can help. We definitely need a windows build guide, now that we added bullet things are getting complicated.

whoa, in cmake? Let me go back and figure that out.

That worked. I thought you guys were referring to a windows environment variable.

After getting the mpg123 and libsnd file and throwing in what looked right, nothing complains.

Let's see if OpenMW builds...

EDIT: Nope. Getting error for bullet includes. "no such file". Gotta go back and try again.

Well, As long as I can make the project, I can just make sure the errors that are due to bullet are the only ones appearing. Long as the errors aren't mine. I can probably start this very shortly...maybe after finals...

Add C:/bullet/src to your additional include directories in visual studio.

Add C:/bullet/src to your additional include directories in visual studio.

Yeah, I guess I could do that. It's not like I'm submitting my own solution files.

CLIFFS: I started, but then realized my process for doing this was silly, so I started over.

Some member variables, particularly in the UI, don't have a very unique name - as a result a simple find a replace project wide results in about 1000+ changes when the reference to the actual variable comes out to be 20 or 30-something. The dangers of this process are being made more clear as I do this. I guess you wanted me to see for myself.

So I started over. My fork was out of date, anyways.

I would love to make some more unique names and add a lot of comments and labels so any task like this becomes easier to do and code become more clear for future users, but that's a different task and may not be received well.

I really am finding distaste in the way VC presents projects. A complex project with so many components and logically separate parts ends up getting thrown into three logical folders for the user. Baffling. It doesn't make the project any easier to navigate or edit and it doesn't make logical sense when you want to work on a specific subsection of it.

As I said in my blog, one of my goals this summer is to really work on my MSVC skills. So if anyone has any tips or things to take a look at or to play with, I'd appreciate it a lot.

Well, you can't say I didn't warn you.

To quote myself from the top of this thread:

Now you need to go through all struct and class definitions and look for member variables, that don't follow the mSomeName scheme, .e.g. when you have "name", it should be replaced by "mName".
You can probably leave alone dumb structs with only public member variables and no functions (even if the member variables there don't have an m-prefix, it hardly matters).

Then compile! Your compiler should provide a large number of error messages, indicating where these variables are used. Fix each one by modifying the name accordingly. Repeat as often as needed.

I suggest you leave libs/openengine and libs/mangle alone for now. These involve submodule handling and require special treatment.

Also, I suggest not to use search and replace or any other kind of automated tool. These are very likely to mess up your code. Letting the compiler find the places requiring change is safer.

Heh - you warned, but I didn't understand the practical significance until I tried it myself.

The problem with compile and fix is that I have to weave through hundreds of warnings and many bullet errors. That makes it very easy to make a mistake or miss a reference completely. I'll try it near the end, of course, but I found that I can right click a given variable and "find all references" and it produces a list of every appearance. When the amount of changes matches the amount of references, that's a good sign.

When it doesn't, I reverse the change and look more carefully.

Sounds like an N^2 algorithm, but it's better than notepad.

Any ETA on this? If possible I would like to see this task completed soon. It seems the number of developers is about to increase significantly in the near future. That also means we will have more concurrent lines of development (a.k.a. branches). If I have to merge in your changes after these lines split of, this can lead to many merge conflicts. Usually it is not a problem, but your changes will be spread out all over the codebase and touch many unrelated lines.

Hmmm...
Well, currently finding a job, so I have free time. If I really wanted, I could hammer away at it. Maybe I should push commits by folder?

I feel bad as I took this on in spring, and had all my time taken by school, then decided to work on my own projects because of the Cmake problems with Bullet.

Let's see - it's Tuesday, so maybe I can do all of apps\openmw\ and get back to you Saturday to see how much is done or left.

One push per folder is a bit of overkill, though one commit per folder seems reasonable. Also, once you have a good chunk of work done you can push if you want, so I can take a look at it.

Just a reminder, so we don't get in trouble. You did remember, that the submodules need special treatment and you should leave them out for now. Right?

One push per folder is a bit of overkill, though one commit per folder seems reasonable. Also, once you have a good chunk of work done you can push if you want, so I can take a look at it.

Just a reminder, so we don't get in trouble. You did remember, that the submodules need special treatment and you should leave them out for now. Right?

Submodules, the renderer, scripts, and all of esmtools are being ignored for this task.

Probably would have been better to name the folders before, I've been figuring it out by conversation and study.

Lemme do this -

I'll do the mwgui folder and then show you, if it's okay, then I'm doing it right and I'll do more.

Okay.

okay -this hasn't happened before - I just commited and got a lot of "Warning: LF will be replaced by CRLF" messages, some in files I never touched. Is there a way I can get a log of a commit?

At any rate - I pushed - most of the private member variables in \mwgui\ now have appropriate prefixes - fixed with some careful checking of all references and replacing only in files I opened myself based off references. Some that looked tricky I left alone and commented for later.

I compiled it, and I only got one instance where a include was changed - that was fixed, recompiled and the log looks the same when I started - no errors in my files except bullet ones and the many, many warnings we usually have.

If this process worked well, I'll continue this process during the week. today I'm done, though. If its wrong I'll paste in your master fork and recommit and push changes back to my fork - or otherwise reset my prefix branch, which doesn't show anyways.

okay -this hasn't happened before - I just commited and got a lot of "Warning: LF will be replaced by CRLF" messages, some in files I never touched. Is there a way I can get a log of a commit?

That is more line ending trouble. I thought we had sorted that out. You can get log messages via git log -n (where n is the number of messages you want starting with the newest). Or you can use git blame to find out who changed which part of a file.

At any rate - I pushed

Sorry, but you did something wrong again. What you pushed to github are some old commits from the beginning of the year. There is nothing new from you on github.

The hell?

Let's see.

I'm on my mprefixtask branch in msysgit. It seems my changes stuck in the source and when I look in the VS project.

if I make a comment, save, and try to switch to master, it doesn't let me...so I obviously committed and pushed to the branch...but...hmmm.

I don't know. Far as I know, I made changes in the correct branch, I've been working and updating only that as time went on - so unless it committed to master, which it couldn't...as far as I know I did it correctly.

Let's take a closer look tomorrow. Today was really not good.

Look what I found it can do:

GITK



I'm done for today. I making a root beer float and reading Claymore 18. Let's fix this tomorrow.

Okay - Good Morning. I have coffee.

From what I can gather so far:

Something is most certainly not right with how I'm set up.

Trying to figure it out on my own, I think if I merged mprefixtask into master, my master branch will have all those changes. That solves the invisible branch problem (and I'll have to learn how to do future task branches without having that happen again).

I am still upset that I could have used a GUI during all this - it would have made things a lot easier. I'm going to learn that interface and see if I can make more sense out it things.

Looking at it - the changes are there, but something isn't right.



Why is mprefixtask yellow? The uncommitted change is a simple comment for testing.

My guess is that you commited your changes but forgot to push them.

I did push them, though. Should I try again?

You did push something. But old commits. I have no idea what is going wrong. This is not supposed to happen.

You did push something. But old commits. I have no idea what is going wrong. This is not supposed to happen.

27-SD event?

Well, it is what it is - Let's see if we can fix it.

What do you think about merging that branch into my master one and just forgetting about the task-branch?

Merge your master branch into your topic branch instead. The resulting branch will be the same, but you are not polluting your master branch with WIP-changes.

Edit: Actually I see a possible source of your problems here. According to github you pushed to your master branch on github. Maybe you even pushed from your local master branch? That would explain why the new changes are not on github.

Merge your master branch into your topic branch instead. The resulting branch will be the same, but you are not polluting your master branch with WIP-changes.

Edit: Actually I see a possible source of your problems here. According to github you pushed to your master branch on github. Maybe you even pushed from your local master branch? That would explain why the new changes are not on github.

Well, I thought about that, but if you have any committed changes on a branch, you get an error telling you to switch to that branch (That is why I have that single comment change hanging there - I wanted to see if I could have done that accidentally).

So - That's how I ruled out being on the master branch when I pushed. Perhaps it doesn't matter which one you're on, but if that were true I would have gotten an error message when I commited then pushed changes on the mprefix branch, since that was the one I've been on since I created it.

I'll try merging master into prefix.

Well, I thought about that, but if you have any committed changes on a branch, you get an error telling you to switch to that branch (That is why I have that single comment change hanging there - I wanted to see if I could have done that accidentally).

That is not correct. You can switch to another branch with committed changes just fine. You only get an error message when you have uncommitted changes.

git checkout mprefixtask

Herp.

git merge master
"Already up to date"

Okay...

You know what? This is actually kind of cool. I'm learning about everything that could go wrong with git, and I'm sure I'll be wiser for it, and so will others, you know, once I resolve it and show them.

AHHHH!

git push origin mprefixtask

=Derp=

branch shows up on github.

So THAT'S how you do it!

Take a look at the private member variables in mwGUI, Zini. I left some alone because I wanted to be careful with them.

I am trying to make sense of it, but I really don't understand what has happened there. First you are working with a badly outdated codebase. You could try to merge in my master branch, but I am not sure if that will work without massive merge conflicts. Also, the commit where you made the first attempt at adding prefixes has a lot of unrelated changes, that make it extremely hard to review and may screw us when I am going to try merge it into my master.

The comment on your commit looks strange. Like a cut of part of a merge message. I am sorry, but I am afraid this branch is not salvageable.

has a lot of unrelated changes,

What? How? Why? Huh?

Okay - let's do this:

I'll erase my branches.
Let's reset my repo completely. I'll carbon copy your remote, and start over.

If the task interferes with the coming release, then we'll simply push the release, and I'll have a fresh master branch to pull from and we'll make it a 0.12.0 task, and it'll be fresh.

That's pretty interesting. Either "Find all references" doesn't find all references or changes snuck in from elsewhere. I'm sad and really frustrated for various reasons, but I'll just start over and do it again - there's no substantial benefit in me doing more than that. If anything, it'll help me stay positive.

From what I see you broke your branch 3 days ago, probably by a really strange combination of a partially failed checkout and a partially failed merge. Both should not be possible though. It would need someone with more git expertise than me to do a post-mortem on this branch.

Starting over is probably the safest choice.

Why is mprefixtask yellow?

Because it is the current HEAD, it is the commit you are currently seeing in your working tree.

Why is mprefixtask yellow?

Because it is the current HEAD, it is the commit you are currently seeing in your working tree.

Just curious: Head suggests a linked list. Does git store/log commits in this way?

Why is mprefixtask yellow?

Because it is the current HEAD, it is the commit you are currently seeing in your working tree.

Just curious: Head suggests a linked list. Does git store/log commits in this way?

Well, the version history is a graph.
Have a look at Git for Computer Scientists

Oh! I know what a graph is! I just never implemented my own.

(yeah, I have to make my own data structures, not allowed to use API or existing structures...)

Interesting.

I destroyed my repo and forked a new fork from zini's fork. I forgot we had our own setup entry in wiki - that actually helped.

I had to move the folder up a directory on my compy (I ended up getting openmw/openmw/), but it seems to be functioning properly now and is up to date. I also am now listed appropriately under Zini's fork on github.

I'm reticent about deleting all my libraries and simply starting over with the environment, but I came too far with the existing ones, I just need to get bullet to appropriately work in the project. I think someone suggested that I move stuff into src last time, and that might have messed it up. I'm not sure.

If anyone has some "from scratch" advice for Bullet, that'd be nice.

Since the opportunity for doing this task without major merging trouble has passed, I suggest we postpone it until 0.14.0 or later.

Since the opportunity for doing this task without major merging trouble has passed, I suggest we postpone it until 0.14.0 or later.

Sounds like a plan.