Timing issues in uzem140

The Uzebox now have a fully functional emulator! Download and discuss it here.
User avatar
Jubatian
Posts: 1355
Joined: Thu Oct 01, 2015 9:44 pm
Location: Hungary
Contact:

Re: Timing issues in uzem140

Post by Jubatian » Mon Oct 19, 2015 4:39 pm

There is something fishy about that merge there. I prepared for a huge lot of conflicts to verify myself, but when I pulled the current master uzem140 branch from Uze, I only found the pixel generation to conflict which was a trivial fix. Nothing else, everything merged together smooth and fine here.

As I checked even that pixel output generation was slightly broken in your merge attempt. Not that it wouldn't work, but it introduced both the 2K linebuffer and kept all the conditionals still there, so ending up with pulling through the negative characteristics of both implementations (original: the branches; the linebuffer: 2K necessary and an AND mask to be safe).

Please check what you did there since really there shouldn't have been more than a few lines of conflict at the pixel output. I checked the rest of the code too, they also seem to be all fine and sound (and of course the resulting emulator works proper here).

User avatar
Artcfox
Posts: 945
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: Timing issues in uzem140

Post by Artcfox » Mon Oct 19, 2015 5:05 pm

Jubatian wrote:There is something fishy about that merge there. I prepared for a huge lot of conflicts to verify myself, but when I pulled the current master uzem140 branch from Uze, I only found the pixel generation to conflict which was a trivial fix. Nothing else, everything merged together smooth and fine here.

As I checked even that pixel output generation was slightly broken in your merge attempt. Not that it wouldn't work, but it introduced both the 2K linebuffer and kept all the conditionals still there, so ending up with pulling through the negative characteristics of both implementations (original: the branches; the linebuffer: 2K necessary and an AND mask to be safe).

Please check what you did there since really there shouldn't have been more than a few lines of conflict at the pixel output. I checked the rest of the code too, they also seem to be all fine and sound (and of course the resulting emulator works proper here).
Apparently I'm not good at resolving these merge conflicts. That was my third attempt, and even that didn't go cleanly, I had to revert some files and start over. I would love to see how you perform merges when there are conflicts. I'll be able to check it later tonight when I get more computer time.

User avatar
Jubatian
Posts: 1355
Joined: Thu Oct 01, 2015 9:44 pm
Location: Hungary
Contact:

Re: Timing issues in uzem140

Post by Jubatian » Mon Oct 19, 2015 6:10 pm

I hope it is not for that there is some monster grinning under the bed here (that is, somehow my merge went foul despite that I did see nothing ill here, neither checking on Github and against the branches to be sure - it shouldn't, after all they manage even the Linux kernel with this thing). Git's normal behavior when merging feels all right for me, it notes the conflicts in the affected files which I can easily check and fix in mcedit. If it is nastier, I just open another console with mcedit to open the original file I am merging against, and maybe another for my original file. 3 x 80 columns fit all right side by side even on my 4:3 display.

I think it is pretty improbable to introduce a merge-related flaw without having some merge conflict, so I mostly trust Git in doing it right where it does (I skim the files through though to be sure).

Maybe a good rule about merges is to try to push the work onto who made the change which have to be merged, after all he should be who is the most familiar with those changes, so it is likely he can fix those up the best (so it is nothing ill in throwing back a mod for that it is not up with the current state of master - I think it ensures that the least potential bugs creep in the process provided the contributor is capable to handle merging all right, but he should be when contributing to a FOSS project).

Anyway, OFF. (I just can't understand what went so messy there)

User avatar
Artcfox
Posts: 945
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: Timing issues in uzem140

Post by Artcfox » Mon Oct 19, 2015 8:26 pm

Jubatian wrote:I hope it is not for that there is some monster grinning under the bed here (that is, somehow my merge went foul despite that I did see nothing ill here, neither checking on Github and against the branches to be sure - it shouldn't, after all they manage even the Linux kernel with this thing). Git's normal behavior when merging feels all right for me, it notes the conflicts in the affected files which I can easily check and fix in mcedit. If it is nastier, I just open another console with mcedit to open the original file I am merging against, and maybe another for my original file. 3 x 80 columns fit all right side by side even on my 4:3 display.

I think it is pretty improbable to introduce a merge-related flaw without having some merge conflict, so I mostly trust Git in doing it right where it does (I skim the files through though to be sure).

Maybe a good rule about merges is to try to push the work onto who made the change which have to be merged, after all he should be who is the most familiar with those changes, so it is likely he can fix those up the best (so it is nothing ill in throwing back a mod for that it is not up with the current state of master - I think it ensures that the least potential bugs creep in the process provided the contributor is capable to handle merging all right, but he should be when contributing to a FOSS project).

Anyway, OFF. (I just can't understand what went so messy there)
One thing I noticed was that git merge wanted to put the part "//draw pixels on scanline" in the wrong spot because the diff failed to account that the update hardware function got split into two, and it wanted to put in above the function that followed, even though that meant that the chunk of code would not be part of a function at all. The automatic merge also seemed to pick the wrong thing in some cases, so I ended up using meld to diff what was there now, versus what was on the remote branch and treating every single line as a merge conflict in order to make sure nothing got silently merged into the wrong spot.

I must be doing something wrong when I find it easier to just look at the diff and re-apply every one of those changes manually.

That's probably a good rule. I'm was just trying to get the merge to work as a "personal improvement" thing, because it's something that I seem to have trouble with, and I figured that if we both tried the merge then we could compare notes at the end to see if they matched, and I could learn what I keep doing wrong.

User avatar
Jubatian
Posts: 1355
Joined: Thu Oct 01, 2015 9:44 pm
Location: Hungary
Contact:

Re: Timing issues in uzem140

Post by Jubatian » Mon Oct 19, 2015 9:21 pm

Artcfox wrote:One thing I noticed was that git merge wanted to put the part "//draw pixels on scanline" in the wrong spot
That's exactly why merging should be left to the inventor! ;) I immediately knew how to resolve that conflict proper (since of course I got the same) as I did that part of the code, and knew exactly what and why went where - so for me it was a matter of minutes to resolve that cleanly, no fuss.

It is a good rule at merging: if you get a conflict, you definitely need to check the section in conflict, and know what the changes occurring there did globally since at other sections they could have introduced changes which merged without conflict but would wreck the code. These obviously are known best by who made the changes.

User avatar
Artcfox
Posts: 945
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: Timing issues in uzem140

Post by Artcfox » Mon Oct 19, 2015 10:42 pm

Jubatian wrote:
Artcfox wrote:One thing I noticed was that git merge wanted to put the part "//draw pixels on scanline" in the wrong spot
That's exactly why merging should be left to the inventor! ;) I immediately knew how to resolve that conflict proper (since of course I got the same) as I did that part of the code, and knew exactly what and why went where - so for me it was a matter of minutes to resolve that cleanly, no fuss.

It is a good rule at merging: if you get a conflict, you definitely need to check the section in conflict, and know what the changes occurring there did globally since at other sections they could have introduced changes which merged without conflict but would wreck the code. These obviously are known best by who made the changes.
Yes, that was part of the problem. Git didn't find a conflict in some parts of the code, and automatically merged things in the wrong way. But since git didn't find a conflict there, it didn't mark it for review, and the merge tool didn't even give me the option to fix it. That is exactly the kind of thing that I would like to learn how to prevent from happening, but isn't that actually impossible to prevent, because Git is just attempting a blind merge without understanding the code? I think that's why I seemed to get better results when I didn't ask Git to merge anything at all, but instead told it to show me a diff of both branches inside a merge tool, and moved things from one side to the other, rearranging as necessary until it made better sense than what Git's automatic merge was suggesting.

And that's why I'm asking how you resolved the merge conflicts in a sane way without having it automatically choose the wrong thing, because there has to be a better way than what I'm trying. (I understand that the merge should be left to the author, I'm just trying to learn this for future reference when that author is me, and it's my job to resolve a conflict.)

User avatar
Jubatian
Posts: 1355
Joined: Thu Oct 01, 2015 9:44 pm
Location: Hungary
Contact:

Re: Timing issues in uzem140

Post by Jubatian » Tue Oct 20, 2015 8:03 am

I rather opened an own topic for this: Merging with Git without a loss of sanity

Post Reply

Who is online

Users browsing this forum: No registered users and 2 guests