From bec2e9b17850c9965185c73a76d5f072670b2c22 Mon Sep 17 00:00:00 2001 From: Maciej Suminski Date: Tue, 3 Dec 2013 15:17:43 +0100 Subject: [PATCH] Added some comments & asserts. --- common/tool/action_manager.cpp | 2 ++ common/tool/tool_manager.cpp | 16 +++++++++++++--- include/tool/tool_event.h | 3 +++ pcbnew/basepcbframe.cpp | 2 +- pcbnew/tools/selection_tool.h | 11 ++++++++++- 5 files changed, 29 insertions(+), 5 deletions(-) diff --git a/common/tool/action_manager.cpp b/common/tool/action_manager.cpp index 63aa99f8d8..d38ad1eccf 100644 --- a/common/tool/action_manager.cpp +++ b/common/tool/action_manager.cpp @@ -44,6 +44,8 @@ ACTION_MANAGER::~ACTION_MANAGER() void ACTION_MANAGER::RegisterAction( TOOL_ACTION* aAction ) { assert( aAction->GetId() == -1 ); // Check if the TOOL_ACTION was not registered before + assert( m_actionNameIndex.find( aAction->m_name ) == m_actionNameIndex.end() ); + assert( m_actionIdIndex.find( aAction->m_id ) == m_actionIdIndex.end() ); aAction->setId( MakeActionId( aAction->m_name ) ); diff --git a/common/tool/tool_manager.cpp b/common/tool/tool_manager.cpp index 2644076508..9fe8715604 100644 --- a/common/tool/tool_manager.cpp +++ b/common/tool/tool_manager.cpp @@ -120,6 +120,11 @@ TOOL_MANAGER::~TOOL_MANAGER() void TOOL_MANAGER::RegisterTool( TOOL_BASE* aTool ) { + wxASSERT_MSG( m_toolNameIndex.find( aTool->GetName() ) == m_toolNameIndex.end(), + wxT( "Adding two tools with the same name may result in unexpected behaviour.") ); + wxASSERT_MSG( m_toolIdIndex.find( aTool->GetId() ) == m_toolIdIndex.end(), + wxT( "Adding two tools with the same ID may result in unexpected behaviour.") ); + TOOL_STATE* st = new TOOL_STATE; st->theTool = aTool; @@ -227,7 +232,10 @@ bool TOOL_MANAGER::runTool( TOOL_BASE* aTool ) wxASSERT( aTool != NULL ); if( !isRegistered( aTool ) ) + { + wxASSERT_MSG( false, wxT( "You cannot run unregistered tools" ) ); return false; + } TOOL_STATE* state = m_toolState[aTool]; @@ -334,10 +342,11 @@ void TOOL_MANAGER::dispatchInternal( TOOL_EVENT& aEvent ) // Go() method that match the event. if( st->transitions.size() ) { - BOOST_FOREACH( TRANSITION tr, st->transitions ) + BOOST_FOREACH( TRANSITION& tr, st->transitions ) { if( tr.first.Matches( aEvent ) ) { + // as the state changes, the transition table has to be set up again st->transitions.clear(); // no tool context allocated yet? Create one. @@ -351,6 +360,9 @@ void TOOL_MANAGER::dispatchInternal( TOOL_EVENT& aEvent ) if( !st->cofunc->Running() ) finishTool( st ); // The couroutine has finished immediately? + + // there is no point in further checking, as transitions got cleared + break; } } } @@ -406,8 +418,6 @@ void TOOL_MANAGER::finishTool( TOOL_STATE* aState ) if( it != m_activeTools.end() ) m_activeTools.erase( it ); - else - wxLogWarning( wxT( "Tried to finish inactive tool" ) ); aState->idle = true; delete aState->cofunc; diff --git a/include/tool/tool_event.h b/include/tool/tool_event.h index 1ec5b1414d..a2815035d2 100644 --- a/include/tool/tool_event.h +++ b/include/tool/tool_event.h @@ -321,6 +321,9 @@ public: if( m_commandId && aEvent.m_commandId ) return *m_commandId == *aEvent.m_commandId; + + // Command-type event has to contain either id or string + assert( false ); } return true; diff --git a/pcbnew/basepcbframe.cpp b/pcbnew/basepcbframe.cpp index 89c5a8866e..dd235976c3 100644 --- a/pcbnew/basepcbframe.cpp +++ b/pcbnew/basepcbframe.cpp @@ -609,7 +609,7 @@ void PCB_BASE_FRAME::UseGalCanvas( bool aEnable ) EDA_DRAW_FRAME::UseGalCanvas( aEnable ); m_toolManager->SetEnvironment( m_Pcb, m_galCanvas->GetView(), - m_galCanvas->GetViewControls(), this ); + m_galCanvas->GetViewControls(), this ); ViewReloadBoard( m_Pcb ); } diff --git a/pcbnew/tools/selection_tool.h b/pcbnew/tools/selection_tool.h index ee8cf45003..faf48ef8db 100644 --- a/pcbnew/tools/selection_tool.h +++ b/pcbnew/tools/selection_tool.h @@ -68,7 +68,16 @@ public: KIGFX::VIEW_GROUP* group; /// Checks if there is anything selected - bool Empty() const { return items.empty(); } + bool Empty() const + { + return items.empty(); + } + + /// Returns the number of selected parts + int Size() const + { + return items.size(); + } }; /// @copydoc TOOL_INTERACTIVE::Reset()