ABToSVK commit

[xonotic/netradiant.git] / docs / developer / Inspector / inspector.txt

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