Transitions, DirectDialog, and DirectRadioButton: changes

As mentioned in here, I’ve made some changes to Transitions, DirectDialog and DirectRadioButton. This thread is intended to get community feedback on those changes.

The altered files, and a small test-program, should be available via this link.

These should be the changes made (presuming that I’ve forgotten nothing, and possibly ignoring some minor changes that can be found via a diff):

• Transitions
  [list]
  []The “fade”-screen is now by default simply a semi-transparent DirectFrame, instead of a model loaded automatically.
  [list][]The Panda-provided model can still be used by loading it manually.
  [/:m]
  []Changed uses of “setColor” and “lerpColorInterval” to “setColorScale” and “lerpColorScaleInterval”, respectively.

• This means that the changes that Transitions makes only tints fade-models, rather than recolouring them as was previously the case. For one, I found that the use of “setColor” was interfering with the “default model” when that model was loaded manually.
  [/:m]
  []A side-effect is that the Panda-provided fade model is now more transparent that it previously was: the model file specifies a degree of transparency (fifty percent, I think it was), which was, I believe, being overwritten by the calls to “setColor”. Using “setColorScale”, both the model’s native transparency and the transparency being specified by the method that shows the fade-screen are being multplied together. However, this is what I would expect to have happen, so I’m inclined to leave it in.[/:m]
  []In “setFadeModel”, added “self.fade.destroy()” before “del self.fade”, as I found that it didn’t seem to be properly cleaned up without the call.[/:m][/list:u][/:m]
  [*]DirectDialog

• The backdrop is now by default a basic grey frame if relief is set to “DGG.FLAT” and invisible otherwise, instead of a model loaded automatically.
  [list]
  [*]The Panda-provided model can still be loaded by setting the dialogue’s “image” constructor keyword to “DGG.getDefaultDialogGeom()”.

• To my mind, while arguably unappealing, the grey frame is at least in keeping with the rest of DirectGUI’s default appearance.
  [/:m]
  []Code has been added to catch two places in which calls to “getTightBounds” may return “None”, potentially tripping up the construction of the dialogue.[/:m]
  []The Panda-provided dialogues (“Yes-No”, “Retry-Cancel”, etc.) now by default show with a flat grey frame.[/:m][/list:u][/:m]
  [*]DirectRadioButton

• The “boxImage” constructor keyword is now by default “None”, instead of an automatically-loaded model.

• If no “boxImage” is specified, DirectRadioButton now defaults to a basic sunken frame.
  [/*:m][/list:u]

Rdb, you mentioned submitting my changes as a patch in the bug-tracker; presuming that they’re found useful, how do go about doing so? Specifically, how do I create a patch, should I create a “bug report” for this, and is there a way of doing so without signing up for Ubuntu One? (My apologies, I don’t think that I’ve done this before–or at least have forgotten it. ^^; )

Ah, this sounds like a nice improvement for elegance and cleanness of the code.

I might be able to help with the last few questions.

I don’t know if this is the cleanest way to create a patch (I’m a git user so I’m not that familiar with creating patches for projects that use other version control systems), but here’s the procedure I have used.

To begin with, you’ll need two copies of the Panda source tree: your modified one, and the original one you want to patch against. It makes things easier to have both as subdirectories under the same parent directory, for example, ~/Documents/panda/panda3d-1.8.1-original and ~/Documents/panda/panda3d-1.8.1-mycustomversion.

The next step is to diff the trees to produce the actual patch. If you use Linux, there is a command-line diff utility that comes with the system (see below for usage).

But before running a diff, it is useful to specify excludes, i.e. filename patterns that won’t be considered for the diff. This is useful to avoid the need to clean the trees before generating the patch, as you can simply exclude the “built” directory and any temporary files from consideration.

You’ll need to do this step only once. Create a file ~/Documents/panda/myexcludes.txt, and put the following lines into it:

built
targetroot
*.pyc
*.deb
*~
*\(*copy\)*

Note that the patterns match both directories and files. Observe that parentheses in file names need to be escaped by prefixing them with “”.

Most of the entries are pretty much self-explanatory; the second-to-last entry excludes the previous (“backup”) version of a file, as kept by many text editors, and the last entry excludes any copies of files created by Nautilus when you Ctrl+C a file and Ctrl+V it while still in the same directory (assuming the OS language is set to English). (I sometimes use this to store temporary intermediate versions, when I’m about to possibly break something.)

If you’re using another OS, be sure to add any filename patterns that correspond to generated files that are not supposed to be part of a clean source tree.

Now that you have the excludes set up, you can run the actual diff, telling it to compare the trees excluding any file or directory names matching those defined in myexcludes.txt. In a terminal, cd ~/Documents/panda and then:

diff -Naur --exclude-from=myexcludes.txt panda3d-1.8.1-original panda3d-1.8.1-mycustomversion >temp.patch

Once the diff completes (it may take a while), open temp.patch in a text editor and examine your changes. If there are accidental changes that shouldn’t be there, modify your custom source to undo those, and re-run the diff.

Once the patch looks good, rename it and send it in.

(There may exist GUI tools to create diffs, but I’m not familiar with those, as I’ve just used the command-line utility myself.)

As for how to send the patches, I’ve used the bug tracker to create a bug (providing a link in the description to the relevant forum thread), and attached the patch to that. You’ll need an Ubuntu One account, though.

Ah, thank you for the information, Technologicat!

Hmm… Would it not be simpler to create diffs on a file-by-file basis (as it seems that my diff-tool (Meld) will do), given that I have only three files to patch?

As to DirectRadioButton, this thread was just posted, and prompted the thought: would it be worth further altering DirectRadioButton such that, if “boxImage” is provided and is a tuple of at least two entries, and “boxGeom” isn’t provided, the system assumes that “boxImage” contains depictions of the “on” and “off” states, and thus doesn’t use the default asterisk? Custom indicators could still be used by specifying “boxGeom”.

Hmm. Meld is indeed very nice for interactive comparison, but it seems its directory comparison mode cannot create patches. It’s weird that this feature has been omitted.

One big diff is the usual approach, since then the patch can be applied with just one application of the patch command-line tool (which is in a sense the inverse of diff - it reads in a patch file, and applies the changes to a source tree).

Maybe it’s possible to create per-file diffs and then concatenate them into the same file*? Have to be careful with directory paths, though, if the files are in different directories.

(* as in: cat file1.patch file2.patch file3.patch >out.patch )

As for DirectRadioButton, I think rdb will have to comment on that

If you are working with a checkout from CVS or Git, you don’t need two copies of the source code. Just run a “git diff” or “cvs diff -u” command and it will give you a patch containing all the changes you made since the latest revision.

Filing a bug requires a Launchpad account currently, sorry.

Git can do this, yes, but I wasn’t aware that CVS has that feature, too. Makes sense, though.

Thanks, as always

Aah, I see–if it’s inconvenient to apply multiple patches, it does look as though I have the Linux diff tool that you mentioned; I wanted to use Meld and individual patches simply for the sake of my own convenience.

By all means comment! I’m interested in getting opinions before submitting–hence the thread.

Aah, that seems to be pretty quick and convenient–thank you!

Fair enough, and thanks.

I think the DirectRadioButton is a fine change. It does break behaviour for people, but it’s likely that they’re already customising the theming anyway.

I prefer separate patches for separate features. This way, they can be reviewed and committed independently. It’s also easier to do that one change at a time. However, if one change spans multiple files, those should go into a single patch.

To create a diff on several files, you can just pass one or more filenames to the “cvs diff” and “git diff” commands.

(Note: cvs diff and git diff are almost identical to the standard “diff” tool in which options they take.)

Fair enough; it’s in for now, then.

Hmm… The changes to Transitions and DirectDialog are technically separate (one can have transitions without dialogues, and while dialogues do use transitions the changes to the former don’t affect the latter), but were made for related reasons…

I think that I’m going to consider them as separate: if you were to keep the changes to Transitions and discard those to DirectDialog there should be no breakages, and vice versa…

The patches should be uploaded (using a relative’s Ubuntu One account) to these “bugs”:
Transitions–changes to fade screen
DirectDialog–removal of model-loading by default and a bug-fix
DirectRadioButton—removal of model-loading by default and indicator handling
(I just noticed that, based on the next entry below mine, I should perhaps have prepended the term “[patch]” to my entries–sorry about that! ^^; )

A new version of the archive should be available via this link. This includes the altered files, a short test/example-program and a simple custom radio-button for the purposes of testing and demonstrating the new indicator behaviour.

I think your suggestion sounds good. From the user’s perspective, it’s definitely a reasonable solution to the problem discussed in the other thread you linked.

I was mainly worried about the break in strict backward compatibility - but since rdb is ok with that, then there should be no problem.

Thank you.

Backward compatibility is an issue, admittedly–but it can eventually become a trap, too, slowing development and hampering attempts to fix issues.

The fix should be in the patch that I logged.