Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sound and other improvements to Sega G-80 games. #7103

Merged
merged 96 commits into from Aug 20, 2020
Merged

Conversation

aaronsgiles
Copy link
Member

Sound and other improvements to Sega G-80 games: [Aaron Giles, couriersud]

  • Added netlist-based sound to Eliminator, Zektor, Space Fury, and Astro Blaster.
  • Split the Sega Universal Sound Board and Speech Boards into their own separate files.
  • Improved Universal Sound Board implementation for better accuracy in Star Trek and Tac/Scan.
  • Wrote netlist-based backend for Universal Sound Board; currently disabled due to limitations in the system.
  • Wrote netlist-based backend for Speech Board; currently disabled pending future sound system changes.
  • Implemented wait states and the vector DRAW flag to help improve timing.

SP0250 Improvements: [Aaron Giles]

  • Matched clock divider to real chip measurements.
  • Fixed behavior when not fed enough data; addresses "gapping" in speech in Sega games.
  • Implemented accurate LFR noise generator according to real chip measurements.
  • Added pulse-width modulation DAC output mode for future consumption by netlist.

Netlist additions: [Aaron Giles]

  • Added compile-time option to record nltool-compatible CSV files.
  • Improved CD4020 implementation.
  • Fixed CD4053 behavior.
  • Added 74139 device.
  • Added TL082 device.

8253 PIT changes: [Aaron Giles]

  • Added explicit synchronization to all writes.
  • Cleaned up some timing calculations to avoid double<->attotime conversions.

aaronsgiles and others added 30 commits July 22, 2020 14:54
…ently stalls on first noise source due to AFUNC interference.
* Removing devices is not a really good idea. Disabled for now. Perhaps
should print some INFO.
aaronsgiles and others added 27 commits August 11, 2020 09:40
…s through speech board. Fix on/off controls.
 * Moved netlist version behind an #ifdef for now, until some tricky
    issues can be resolved.
 * HLE code now runs at full fidelity, fixing Tac/Scan sounds.
 * HLE shot sounds
 * add option to simplify SONAR sounds
 * updated documentation on current issues
* This is a short term solution. A NET_MODEL_OVERWRITE directive will be
added to netlist later.
@aaronsgiles aaronsgiles merged commit f7b263d into master Aug 20, 2020
@aaronsgiles aaronsgiles deleted the aaron-nl2 branch August 22, 2020 22:05
Comment on lines +105 to +108
save_item(NAME(m_filter[index].F), index);
save_item(NAME(m_filter[index].B), index);
save_item(NAME(m_filter[index].z1), index);
save_item(NAME(m_filter[index].z2), index);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use STRUCT_MEMBER to help save these and avoid clutter in the debugger?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, didn't know about it and there's fairly few examples of it in the code.

Comment on lines +451 to +456
{
// FIXME: Add an directive MODEL_OVERWRITE to netlist language
//throw nl_exception(MF_MODEL_ALREADY_EXISTS_1(model_in));
log().info(MI_MODEL_OVERWRITE_1(model, model_in));
m_abstract.m_models[model] = def;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a good idea? Being able to overwrite a model has potential to wreak havoc, and building things that depend on it sounds like a workaround for some other issue. I think @couriersud had it right previously in preventing models from being overwritten.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a commit from @couriersud directly as a workaround for limitations in the current model situation. I believe he intends to support a cleaner mechanism in the future.

@@ -0,0 +1,1225 @@
// license:BSD-3-Clause
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weren’t we trying to standardise on CC0 licensing for netlists? An actual description of the circuit doesn’t qualify for copyright protection. The only things that are potentially protected by copyright are the organisation/formatting of the file, and comments that contain sufficient creative work. @couriersud made an effort to get the netlists contributed by MAME developers made available under CC0 some time ago, but recently several have been added with BSD-3-Clause.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you were, that's news to me, but I don't care as long as it's not GPLx.

Comment on lines +531 to +534
// unconnected inputs to analog switch -- what's the right approach?
NET_C(GND, U8.2, U8.5, U7.2)
NET_C(GND, U15.2, U15.5, U16.2)
NET_C(GND, U30.2, U30.5, U31.2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they’re not connected to anything, they’ll look like a high-impedance path to the substrate, not a low-impedance ground connection.

Comment on lines +33 to +35
u8 m_lo_mask;
u8 m_hi_mask;
bool m_has_psg;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren’t these const? They should only be set on construction, and bad things will happen if they get modified while running.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Also the netlist and scale factor.


#pragma once

#define ENABLE_NETLIST_FILTERING (0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is far too generic for a macro that ends up in a header that could get included before other headers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. It doesn't need to be in the header anyway so I'll just move it into the cpp file.

class segag80_audio_device : public device_t, public device_mixer_interface
{
public:
segag80_audio_device(const machine_config &mconfig, device_type type, const char *tag, device_t *owner, u32 clock, u8 lomask, u8 himask, bool haspsg, void (*netlist)(netlist::nlparse_t &), double output_scale);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constructor should only be called by derived classes, it should not be public (and the class should possibly be called segag80_audio_device_base to indicate that it’s an abstract base, not a concrete device implementation).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair.

Comment on lines +15 to +16
#include "includes/segag80r.h"
#include "includes/segag80v.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you start needing the driver header in a device used in the driver, you’ve created circular dependencies. Is there a something you can do to avoid this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't needed; probably vestigial from when I split segasnd.

Comment on lines +136 to +160
for (int tgroup = 0; tgroup < 3; tgroup++)
{
timer8253 &group = m_timer_group[tgroup];
for (int tchan = 0; tchan < 3; tchan++)
{
timer8253::channel &channel = group.chan[tchan];
save_item(NAME(channel.holding), tgroup * 3 + tchan);
save_item(NAME(channel.latchmode), tgroup * 3 + tchan);
save_item(NAME(channel.latchtoggle), tgroup * 3 + tchan);
save_item(NAME(channel.clockmode), tgroup * 3 + tchan);
save_item(NAME(channel.bcdmode), tgroup * 3 + tchan);
save_item(NAME(channel.output), tgroup * 3 + tchan);
save_item(NAME(channel.lastgate), tgroup * 3 + tchan);
save_item(NAME(channel.gate), tgroup * 3 + tchan);
save_item(NAME(channel.subcount), tgroup * 3 + tchan);
save_item(NAME(channel.count), tgroup * 3 + tchan);
save_item(NAME(channel.remain), tgroup * 3 + tchan);
}
save_item(NAME(group.env), tgroup);
save_item(NAME(group.chan_filter[0].capval), tgroup);
save_item(NAME(group.chan_filter[1].capval), tgroup);
save_item(NAME(group.gate1.capval), tgroup);
save_item(NAME(group.gate2.capval), tgroup);
save_item(NAME(group.config), tgroup);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use STRUCT_MEMBER for saving members of structures in arrays?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

{
LOG("%s:usb_data_r = %02X\n", machine().describe_context(), (m_out_latch & 0x81) | (m_in_latch & 0x7e));

m_maincpu->adjust_icount(-200);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should guard against disabled side effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants