fix item hilighting in debugger state view when scrolling [Vas Crabb]

better encapsulate debug_view_state::state_item

also check for debugger access for reads with side effects in osborne1 and s2636

src/devices/machine/s2636.cpp

@@ -414,7 +414,8 @@ READ8_MEMBER( s2636_device::read_data )
 	{
 	case REG_COL_BG_CMPL:
 	case REG_VBL_COL_OBJ:
-		m_registers[offset] = 0x00; // collision/completion/VRESET flags reset on read
+		if (!space.debugger_access())
+			m_registers[offset] = 0x00; // collision/completion/VRESET flags reset on read
 		break;
 	}
 

src/emu/debug/dvstate.cpp

@@ -28,9 +28,9 @@ const int debug_view_state::REG_FRAME;
 //-------------------------------------------------
 
 debug_view_state_source::debug_view_state_source(const char *name, device_t &device)
-	: debug_view_source(name, &device),
+	: debug_view_source(name, &device)
-		m_stateintf(dynamic_cast<device_state_interface *>(&device)),
+	, m_stateintf(dynamic_cast<device_state_interface *>(&device))
-		m_execintf(dynamic_cast<device_execute_interface *>(&device))
+	, m_execintf(dynamic_cast<device_execute_interface *>(&device))
 {
 }
 
@@ -45,9 +45,9 @@ debug_view_state_source::debug_view_state_source(const char *name, device_t &dev
 //-------------------------------------------------
 
 debug_view_state::debug_view_state(running_machine &machine, debug_view_osd_update_func osdupdate, void *osdprivate)
-	: debug_view(machine, DVT_STATE, osdupdate, osdprivate),
+	: debug_view(machine, DVT_STATE, osdupdate, osdprivate)
-		m_divider(0),
+	, m_divider(0)
-		m_last_update(0)
+	, m_last_update(0)
 {
 	// fail if no available sources
 	enumerate_sources();
@@ -147,7 +147,7 @@ void debug_view_state::recompute()
 	{
 		count++;
 		maxtaglen = (std::max)(maxtaglen, item.m_symbol.length());
-		maxvallen = (std::max)(maxvallen, item.m_vallen);
+		maxvallen = (std::max)(maxvallen, item.value_length());
 	}
 
 	// set the current divider and total cols
@@ -186,125 +186,108 @@ void debug_view_state::view_update()
 		recompute();
 
 	// get cycle count if we have an execute interface
-	const debug_view_state_source &source = downcast<const debug_view_state_source &>(*m_source);
+	debug_view_state_source const &source(downcast<debug_view_state_source const &>(*m_source));
-	UINT64 total_cycles = 0;
+	UINT64 const total_cycles(source.m_execintf ? source.m_execintf->total_cycles() : 0);
-	if (source.m_execintf != nullptr)
+	bool const cycles_changed(m_last_update != total_cycles);
-		total_cycles = source.m_execintf->total_cycles();
 
-	// find the first entry
+	// loop over rows
-	auto it = m_state_list.begin();
+	auto it(m_state_list.begin());
-	for (INT32 index = 0; (index < m_topleft.y) && (it != m_state_list.end()); ++index, ++it) { }
+	screen_device const *const screen(machine().first_screen());
-
+	debug_view_char *dest(&m_viewdata[0]);
-	// loop over visible rows
+	for (INT32 index = 0, limit = m_topleft.y + m_visible.y; (index < limit) || (it != m_state_list.end()); ++index)
-	screen_device *screen = machine().first_screen();
-	debug_view_char *dest = &m_viewdata[0];
-	for (UINT32 row = 0; row < m_visible.y; row++)
 	{
-		UINT32 col = 0;
+		bool const visible((index >= m_topleft.y) && (index < limit));
+		UINT32 col(0);
 
-		// if this visible row is valid, add it to the buffer
 		if (it != m_state_list.end())
 		{
-			state_item &curitem = *it;
+			// advance to the next item
+			state_item &curitem(*it++);
 
-			UINT32 effcol = m_topleft.x;
+			// update item and get the effective string
-			UINT8 attrib = DCA_NORMAL;
-			UINT32 len = 0;
 			std::string valstr;
-
+			switch (curitem.index())
-			// get the effective string
-			if (curitem.m_index >= REG_FRAME && curitem.m_index <= REG_DIVIDER)
 			{
-				curitem.m_lastval = curitem.m_currval;
+			case REG_DIVIDER:
-				switch (curitem.m_index)
+				curitem.m_symbol.clear();
+				curitem.m_symbol.resize(m_total.x, '-');
+				break;
+
+			case REG_CYCLES:
+				if (source.m_execintf)
 				{
-				case REG_DIVIDER:
+					curitem.update(source.m_execintf->cycles_remaining(), cycles_changed);
-					curitem.m_vallen = 0;
+					valstr = string_format("%-8d", curitem.value());
-					curitem.m_symbol.clear();
+				}
-					for (int i = 0; i < m_total.x; i++)
+				break;
-						curitem.m_symbol.append("-");
-					break;
 
-				case REG_CYCLES:
+			case REG_BEAMX:
-					if (source.m_execintf != nullptr)
+				if (screen)
-					{
+				{
-						curitem.m_currval = source.m_execintf->cycles_remaining();
+					curitem.update(screen->hpos(), cycles_changed);
-						valstr = string_format("%-8d", UINT32(curitem.m_currval));
+					valstr = string_format("%4d", curitem.value());
-					}
+				}
-					break;
+				break;
 
-				case REG_BEAMX:
+			case REG_BEAMY:
-					if (screen != nullptr)
+				if (screen)
-					{
+				{
-						curitem.m_currval = screen->hpos();
+					curitem.update(screen->vpos(), cycles_changed);
-						valstr = string_format("%4d", UINT32(curitem.m_currval));
+					valstr = string_format("%4d", curitem.value());
-					}
+				}
-					break;
+				break;
 
-				case REG_BEAMY:
+			case REG_FRAME:
-					if (screen != nullptr)
+				if (screen)
-					{
+				{
-						curitem.m_currval = screen->vpos();
+					curitem.update(screen->frame_number(), cycles_changed);
-						valstr = string_format("%4d", UINT32(curitem.m_currval));
+					valstr = string_format("%6d", curitem.value());
-					}
+				}
-					break;
+				break;
 
-				case REG_FRAME:
+			default:
-					if (screen != nullptr)
+				curitem.update(source.m_stateintf->state_int(curitem.index()), cycles_changed);
-					{
+				valstr = source.m_stateintf->state_string(curitem.index());
-						curitem.m_currval = screen->frame_number();
+			}
-						valstr = string_format("%6d", UINT32(curitem.m_currval));
+
-					}
+			// if this row is visible, add it to the buffer
-					break;
+			if (visible)
+			{
+				// see if we changed
+				const UINT8 attrib(curitem.changed() ? DCA_CHANGED: DCA_NORMAL);
+
+				// build up a string
+				char temp[256];
+				UINT32 len(0);
+				if (curitem.m_symbol.length() < (m_divider - 1))
+				{
+					memset(&temp[len], ' ', m_divider - 1 - curitem.m_symbol.length());
+					len += m_divider - 1 - curitem.m_symbol.length();
+				}
+
+				memcpy(&temp[len], curitem.m_symbol.c_str(), curitem.m_symbol.length());
+				len += curitem.m_symbol.length();
+
+				temp[len++] = ' ';
+				temp[len++] = ' ';
+
+				memcpy(&temp[len], valstr.c_str(), curitem.value_length());
+				len += curitem.value_length();
+
+				temp[len++] = ' ';
+				temp[len] = 0;
+
+				// copy data
+				for (UINT32 effcol = m_topleft.x; (col < m_visible.x) && (effcol < len); ++dest, ++col)
+				{
+					dest->byte = temp[effcol++];
+					dest->attrib = attrib | ((effcol <= m_divider) ? DCA_ANCILLARY : DCA_NORMAL);
 				}
 			}
-			else
-			{
-				if (m_last_update != total_cycles)
-					curitem.m_lastval = curitem.m_currval;
-				curitem.m_currval = source.m_stateintf->state_int(curitem.m_index);
-				valstr = source.m_stateintf->state_string(curitem.m_index);
-			}
-
-			// see if we changed
-			if (curitem.m_lastval != curitem.m_currval)
-				attrib = DCA_CHANGED;
-
-			// build up a string
-			char temp[256];
-			if (curitem.m_symbol.length() < m_divider - 1)
-			{
-				memset(&temp[len], ' ', m_divider - 1 - curitem.m_symbol.length());
-				len += m_divider - 1 - curitem.m_symbol.length();
-			}
-
-			memcpy(&temp[len], curitem.m_symbol.c_str(), curitem.m_symbol.length());
-			len += curitem.m_symbol.length();
-
-			temp[len++] = ' ';
-			temp[len++] = ' ';
-
-			memcpy(&temp[len], valstr.c_str(), curitem.m_vallen);
-			len += curitem.m_vallen;
-
-			temp[len++] = ' ';
-			temp[len] = 0;
-
-			// copy data
-			while (col < m_visible.x && effcol < len)
-			{
-				dest->byte = temp[effcol++];
-				dest->attrib = attrib | ((effcol <= m_divider) ? DCA_ANCILLARY : DCA_NORMAL);
-				dest++;
-				col++;
-			}
-
-			// advance to the next item
-			++it;
 		}
 	
 		// fill the rest with blanks
-		while (col < m_visible.x)
+		while (visible && (col < m_visible.x))
 		{
 			dest->byte = ' ';
 			dest->attrib = DCA_NORMAL;
@@ -330,3 +313,15 @@ debug_view_state::state_item::state_item(int index, const char *name, UINT8 valu
 	, m_symbol(name)
 {
 }
+
+
+//-------------------------------------------------
+//  update - update value and save previous
+//-------------------------------------------------
+
+void debug_view_state::state_item::update(UINT64 newval, bool save)
+{
+	if (save)
+		m_lastval = m_currval;
+	m_currval = newval;
+}

src/emu/debug/dvstate.h

@@ -50,19 +50,30 @@ protected:
 	virtual void view_notify(debug_view_notification type) override;
 
 private:
-	struct state_item
+	class state_item
 	{
+	public:
 		state_item(int index, const char *name, UINT8 valuechars);
 		state_item(const state_item &) = default;
 		state_item(state_item &&) = default;
 		state_item &operator=(const state_item &) = default;
 		state_item &operator=(state_item &&) = default;
 
-		UINT64              m_lastval;          // last value
+		UINT64 value() const { return m_currval; }
-		UINT64              m_currval;          // current value
+		bool changed() const { return m_lastval != m_currval; }
-		int                 m_index;            // index
+		int index() const { return m_index; }
-		UINT8               m_vallen;           // number of value chars
+		UINT8 value_length() const { return m_vallen; }
-		std::string         m_symbol;           // symbol
+
+		void update(UINT64 newval, bool save);
+
+	private:
+		UINT64      m_lastval;          // last value
+		UINT64      m_currval;          // current value
+		int         m_index;            // index
+		UINT8       m_vallen;           // number of value chars
+
+	public:
+		std::string m_symbol;           // symbol
 	};
 
 	// internal helpers

src/mame/machine/osborne1.cpp

@@ -102,15 +102,18 @@ WRITE8_MEMBER( osborne1_state::videoram_w )
 
 READ8_MEMBER( osborne1_state::opcode_r )
 {
-	// Update the flipflops that control bank selection and NMI
+	if (!space.debugger_access())
-	UINT8 const new_ub6a_q = (m_btn_reset->read() & 0x80) ? 1 : 0;
-	if (!m_rom_mode)
 	{
-		set_rom_mode(m_ub4a_q ? 0 : 1);
+		// Update the flipflops that control bank selection and NMI
-		m_ub4a_q = m_ub6a_q;
+		UINT8 const new_ub6a_q = (m_btn_reset->read() & 0x80) ? 1 : 0;
+		if (!m_rom_mode)
+		{
+			set_rom_mode(m_ub4a_q ? 0 : 1);
+			m_ub4a_q = m_ub6a_q;
+		}
+		m_ub6a_q = new_ub6a_q;
+		m_maincpu->set_input_line(INPUT_LINE_NMI, m_ub6a_q ? CLEAR_LINE : ASSERT_LINE);
 	}
-	m_ub6a_q = new_ub6a_q;
-	m_maincpu->set_input_line(INPUT_LINE_NMI, m_ub6a_q ? CLEAR_LINE : ASSERT_LINE);
 
 	// Now that's sorted out we can call the normal read handler
 	return m_maincpu->space(AS_PROGRAM).read_byte(offset);