UI incorrectly allows add/delete groups and materials in animation mode

[zturtleman]

In animation mode the Model class rejects add/delete but Edit Groups and Edit Materials (and possibly others, projections?) still add or deletes items from list. The changes are lost after closing the window since it's not saved in Model.

addGroup() etc return -1 on failure but it's not checked. I made deleteGroup() etc return false in 7c20fc2a310ba36c3cd817d155c03f0928ee01f2 but it's only checked in Edit Joints window. Though a higher level issue is the buttons should be disabled or show a message box. For "add" buttons a message box is used to get name before addGroup( name ) etc is called so there needs to be another way to check if 'editing' is allowed. I'd rather not tie it to animation mode everywhere.

[m-7761]

FWIW every little change generates an Undo record. In the Group editing window there are two sliders that generate Undo records in real-time as the sliders are dragged. Either A) it should be possible to opt out of Undo records. Or B) if the record on the back of the list matches the new record's type they should be merged. Merging seems best...

It might be naive in some instances. But maybe those should be fixed by manually overriding the automatic merging behavior on a case by case basis.

It's a problem that changing the model is so tightly coupled to undo/redo. So it needs attention. It might even such that dragging vertices is generating hundreds of undo records, just based on this one example. If the vertices are subjected to the same criteria, then probably so.

[m-7761]

P.S. I think there's actually a lot of logic problems in GroupWin's case. I can't begin to list them, but for one groupSelectedEvent and textureSelectedEvent use modelAnimate that is strictly of use to AnimWin as near as I can tell. I think groupSelectedEvent has no business using it. And the other should use the more common modelUpdated. EDITED: Removed second observation, realizing it was off base. There are minor issues too, but I can't bring myself to write them down. I think my audit/rewrite will shake out a lot of these issues. There are issues everywhere.

FWIW DecalManager::getInstance()->modelUpdated(model); lines appear all over the place. I'm sure it's something that shouldn't be implemented that way. I think it amounts to repainting the scene. I still have no clue what a "decal" means no matter how many times I see that.

(I just happened to be working on this Group window today. I finished the sidebar yesterday. It was well more than 20 files. Now it's 2 compact, medium sized files. Probably 70 to 90% less code.)

[m-7761]

This may be related. I might be wrong, but I believe these "events" are not hooked up correctly, and so m_canEdit = false; never occurs. That bool is passed as a pointer to all of the "Command" objects, meaning that they will always edit.

It's related to the animation system, but I think it never emits that "signal" or something. Whether it's a cause of the problem or not, it's the same situation I believe, as it pertains to the "tools".

EDITED: I'm not in a position (yet) to trace these. I'm just going by how things appear to me, looking over the code.

/*UNUSED
void MainWin::editDisableEvent() 
{
    m_canEdit = false;
    view_panel.setEnabled(m_canEdit);
}*/
/*UNUSED? (I think if editDisableEvent is UNUSED this is meaningless, though used.)
void MainWin::editEnableEvent() 
{
    m_canEdit = true;
    view_panel.setEnabled(m_canEdit);
}*/

[m-7761]

Follow-up:

Sorry to pile on...

Actually it seems like "view_panel.setEnabled" would be more dramatic than just opening/starting the animation mode subwindow. I may be wrong, or my code may be newer than the newest release's.

  if(!*m_canEdit)
  {
      model_status(model,StatusError,STATUSTIME_LONG,
      ::tr("You are in animation mode, but there are no animations"));
      return;
  }

It seems this is limited to "animation mode, but there are no animations" but that seems like such an odd case to single out. I can make the current release enter this mode. It disables the zoom feature and view selection menus. I can't see a benefit myself.

[m-7761]

FWIW every little change generates an Undo record. In the Group editing window there are two sliders that generate Undo records in real-time as the sliders are dragged. Either A) it should be possible to opt out of Undo records. Or B) if the record on the back of the list matches the new record's type they should be merged. Merging seems best...

It might be naive in some instances. But maybe those should be fixed by manually overriding the automatic merging behavior on a case by case basis.

It's a problem that changing the model is so tightly coupled to undo/redo. So it needs attention. It might even such that dragging vertices is generating hundreds of undo records, just based on this one example. If the vertices are subjected to the same criteria, then probably so.

For the record, there is some after-the-fact Undo combination logic built into the Undo class. See Undo::combine.

Off-topic: MU_SubdivideTriangle::redo isn't implemented. Don't know why, or if that is common. EDITED: As an afterthought, it might invoke operationComplete("Doomed operation");. But that's not documented if so.

I've found myself removing std::list from the bulk of the code. Kevin (one assumes) favored list and front and pop_front where inappropriate. I think if deleting geometry is slow it's because of pathological use of list. Like the various s_recycle objects probably have more overhead than if the memory was not recycled. That is, the solution to memory management overhead entails more memory management than the memory itself would.

[m-7761]

EDITED: The combine functionality is done at addUndo. Unfortunately the Model API uses Model::sendUndo, usually disabling the feature. That isn't completely bad, except I believe the caller should be controlling this, and "operationComplete" should always try to combine, because an "operation" is a single Ctrl+Z, and therefor stands nothing to lose from consolidation.

[m-7761]

For the record I've removed all of this blocking code from the core API (although I should try to double-check) in my working code I should publish before too long. That includes the ability to add/delete geometry. (I worked on that last week since I couldn't add joints in skeletal mode, and realized that the restriction shouldn't apply to any of the Tools.)

That just leaves #87 which I'm thinking about a quick fix for now.