OK. Again I would have liked to get a design document before it being done. Main functionalities we need in the inspector: - Unifiy the inspector under a single dialog box, called with 'S' - Depending on what is currently selected, display several frames in the inspector: only brushes -> surface inspector only patches -> patch inspector brushes & patches -> both and later when brush primitives are mixed with regular brushes + plugin entities, raise whatever additional inspector stuff we need - The camera view must update realtime when we change some parameters. - Get rid of the Apply button, use the Undo code to store settings when surface inspector is raised. If user hits Cancel, call the undo stuff. - Use the message broadcasting stuff to keep the inspectors up to date when the user changes the current selection. Be careful to keep the undo stuff in sync with the select / deselect operations. - Use a 3-state scheme to display the params in the widgets. If two faces are selected that don't have the same shift increment, just grey out the shift box. Messaging: - a good chunk of the work is moving the selection/creation stuff to the messaging API we no longer use UpdateSurfaceDialog, we post messages instead .. the surface inspector has hooked one of it's listeners into the corresponding message we may need to reorganize the messages, maybe introduce a hierarchy? or pass a void * param with messages? - we don't post messages like "update surface inspector", we post messages that say "this and that have changed", then the surface inspector reacts if it needs to. Do we need marshalling in the messages? Very likely .. maybe using Gtk signal stuff would be interesting? -> the messaging stuff is a big chunk of work and our surface inspector changes are not totally dependent on it. Better leave that for l8r the inspector works by states and transitions? Or we post messages to it? Use case: the user raises the inspector .. if we are up we'll ignore, if we are hidden we'll go through the whole process (initialise, look at what is selected, display) then we enter an active state (listening for select / deselects and applying stuff) all in all it seems to be too big a change for next release. will see later probably. Trying a few more days with it, see what happens. after all the interface is fairly restricted so there's a good chance our changes are fairly stable in the end. But rebuilding the whole interface part might be too much ... We need something state based? AND a set of messages .. but first, need to write the initialisation loop build the dialog, get the current surface information and display Undoing the changes on the selected stuff: at any point in time, one can get a snapshot of selected stuff and use it to store the surface properties settings for later on. But what happens if the user modifies the selected brush, pushing it in the undo stack? Then we would cancel the changes? (and just backup to the state right after the modif) We could has the 'Apply' button used for that .. grey it out when the current state is the one in the backup. This happens whenever we hit 'Apply' or change something in the selection. The selection has several items: entities, brushes and selected faces (possibly later generic plugin entities) Current undo stuff is aimed at entities and brushes. NOTE: you can't have selected faces and brushes/entities at the same time, that's a good point to keep that seperated to deal with undo and storage On what side should the implemetation be ? undo.cpp select.cpp or surfacedialog.cpp ? We are going to do it with the messaging API anyway.. And hook in the undo stuff, to reset the snapshot each time something gets pushed in the undo? We have advanced stuff on the Inspector branch, doing basics on Alpha branch. Start writing the watch code in surfacedialog.cpp, see if we need some merging with Undo stuff l8r We need to track for the patch inspector as well.. basic code for CSurfaceUndo written. need to add hooks for the snapshot stuff and undo stuff. and a debug flag to monitor the life cycle of the object. some use cases: - select a brush - bring up surface inspector - check we had the debug messages from CSurfaceUndo (initialise, activate, snapshot) - edit the surface settings - check the views are updating correctly - hit Ok - check we had a deactivate message OK - select a brush - bring up surface inspector - check we had the debug messages from CSurfaceUndo (initialise, activate, snapshot) - edit the surface settings - check the views are updating correctly - hit cancel / escape - check we have a undo and deactivate from CSurfaceUndo OK - select a brush - bring up the surface inspector - edit the surface settings - hit apply - edit them again - hit cancel / escape - check you get back to the apply state OK - make two brushes - select a brush - bring up surface inspector - change settings - select an additional brush - check the surface inspector, new snapshot - hit cancel - check brushes remained in the same state - use standard Undo - check the first brush got back to it's initial settings OK - select a brush - bring up surface inspector - change settings - select an additional brush - check the surface inspector, new snapshot - change more settings - hit cancel - check the first brush returned to intermediate state, and second to initial state (i.e. last snapshot) OK g_surfaceUndo acts as a layer on top of the core Undo code when the surface inspector is activated. We need it because the surface inspector can edit faces which are not handled by the undo? (or does the current code push the whole brush when editing a face?) not sure of the utility of the g_surfaceDialog hooks here .. default undo usage in the sruface inspector sends way too many undo messages. with the new scheme we store in undo only when select/deselect or user hits apply that way the 'Cancel' and later Ctrl+Z calls make sense but is it worth implementing a new class to achieve that?? .. yes because we intend a later cleanup of this part. (ahem is this reason good enough..) this part is actually much closer from the undo code than I had expected.. 'Cancel' call being an Undo call.. going to Inspector3: don't create a new class, simply use the Undo more intelligently? i.e. don't create undo stuff when editing the brush -> we add a flag to turn off the default undo behaviour and force Undo storage when we want we could also store the undo Id we are interested in and call undo several times to get it back NOTE: what happens if the user hits undo when the surface inspector is up? -> we'll have to take his request into account? err .. performing which undo? The texture positioning or something else? seems the snapshot approach would still make sense then? more use cases, see with Undo calls and select/deselect events NOTE: this whole thing is probably a single call to select_settexture that needs to be turned on/off instead of working at the undo level. but we would like to move to messaging so maybe it still makes sense the undo call is in Select_SetTexture (which does not have that many callers, I was expecting more) the question about having the undo code keep working when surface inspector is around is still raised. but it makes it a lot harder, gotta have a real inspector mode in the undo? dunno, think about it again later two operations are mixed in a single one and should not be: reading the map to get the current data we'll manipulate feed it in the dialog box widgets WARNING: when putting stuff in the widgets, it raises a shitload of update messages and therefor completely fucks up our OnOK OnApply OnCancel scheme (specially OnApply!) NOTE: we want to switch between Surface inspector for brushes only and Patch inspector for patches only there's some crappy code in the surface inspector that we need to get rid of but need to check about that before with Spog or others Forcing the way into using the surface inspector is SCREWED? Doesn't seem to work the way we want to. Always get parasite Undo messages and stuff. We could use a seperate stack for Undo with the surface inspector? Just store the surface properties in a seperate stack? When user hits cancel you go back and apply whatever you had? Doesn't seem like a clean way either. Now dealing with both regular surface inspector and patch inspector: we have some stuff that needs to be on/off with the two inspectors what about catching the messages and issuing new snapshots? the main surface inspector is doing it? no! so what, we have several states? FUCKED UP INSPECTOR 5 ---------------------------------------------------------------- restarted from scratch, made much more simple changes. trying another trick for undo (!) just let the undo work as usual, but call undo ourselves in SetTexMods if we have create the last do requires proper initialization/deinitialisation.. in SetTexMods and GetTexMods.. getting rid of patch manipulation code in the regular surface inspector. The buttons will still work, but manip will require the patch inspector. (seems the patch inspector doesn't have that much success anyway) TODO: OK get rid of patch stuff OK get rid of the texture toolbar? (it's broken right now) (and doesn't have anything usefull..) OK (Partial) OnCancel? we need to cancel the texdef as well store an undo texdef each time we grab new texdef stuff this works in reverse than the Undo code? When we do the initial problem is, in some cases the settings that show up are not in sync with what's in the inspector?? (we can't avoid that because if a brush is selected there's no single setting) prolly get it out as is and let Spog or others send feedback about what it's supposed to do.. for now: store stuff in the cancel texdef when we initialize an undo loop revert to that if OnCancel is used OK message when spinning over a patch? DUPLICATE (.. see below ..) check the increments we store in the SI are used when shift + arrows etc. no it doesn't work .. the shifting on keyboard shortcuts is done with m_nTextureTweak seems m_nTextureTweak is nowhere available in the prefs (and it's not in MFC builds either) some cleanup to be done around that it seems OK (.. merged with below, maybe some special cases left ..) texture widget (catch the Enter key to force-call an OnApply) OK (.. see above ..) catch Enter key at dialog level to call OnDone NO (.. it's clean, but thats too many lines of code ..) move the code that blokes updates to use gtk_signal_handler_block_by_func and gtk_signal_handler_block_by_func OK shift + arrow must match the SI settings, OK (FIXME .. not using the right scale (using the scale step instead! + add a button in SI to 'Match grid') POSTPONED (.. m_nTextureTweak is used in the nudge commands .. .. and nudge shortcuts are broken right now ..) get rid of m_nTextureTweak + SI and PI always on top! + known issues: "Match Grid" is broken in BP mode now on the patch inspector (nightmare!): OK (.. put it as readonly .. don't bother ..) texture name widget is screwed? OK the spinners scheme doesn't work, the stuff in the dialog is the inc step and we just need arrows OK get rid of the 'Type' dialog box POSTPONED (.. can't do undo on PI without proper Undo module ..) add proper Done Apply Cancel with Undo NO (.. too much work for something that sucks ..) make the changes reflect in the views when manipulating the entries OK (.. using %g ..) cut down on the number of digits! OK increment steps to be stored in the registry putting the Cancel stuff in the surface inspector: only based on the Undo code, no cancel settings to store because we don't have actual storage of a current texdef (we only send alterations) BTW we should do that for brushes as well the patch inspector works by increments, Patch_SetTextureInfo to incrementally modify the patch. we can still do some undo by having a texdef storing the changes and working together with the undo if the undo is recognized, it means our current texdef increment is valid no, we can't represent the combination of several increments scale and rotate in a single texdef.. get rid of the undo code for now .. only Apply and Done left it seems it's still vastly broken when you select something. or is it on linux only? need a LOT of testing and figuring it out!! selecting a brush breaks totally.. (the texture screws up it seems) does it attempt to change the texture of the selected object?? also: it seems you can multiple select a same brush?? the UNDO code of the SURFACE INSPECTOR IS STILL BROKEN ???? (ok I'm really screwed, time to sleep) -> can't reproduce now?? maybe it's linux specific problem, I can't tell FOUND A WAY TO REPRODUCE THE CRASH: + select brush + hit "Fit" + hit the shift spinners two times OR: + select single face on brush + manually edit scale values -> maybe we have a problem with current texture? (NO) it's some kind of infinite loop? we call UpdateSurfaceInspector from Select_Brush and bang! no, it's a texdef from a face that got deleted prolly that hooking the undo code in there screws up the selected faces stuff if you undo a selected face operation, you end up with the whole brush selected. but that does not necessarily explain why you remove the face at Undo_Start ho well .. removed the undo buffering when selected faces and everything's better would need to re-establish the right face selection after undo, might solve the problem (actually you'd still need to have the settings point to the right object) From PJ about the 'Match Grid' stuff: textures are moved in pixels, not units. We must rely on the current texture scale AND gridsize to compute the shift increment