update to add network for Sega X-Board, Y-Board, Konami K056230, Namco C139
This includes updated socket handling - accepting an incoming socket could be mistaken for end of stream; this has been changed so that accepting returns a would_block error.
Sega X-Board comms make 'Super Monaco GP', and 'Last Survivor' playable in LAN.
Sega Y-Board comms make 'Power Drift - Link Version' playable in LAN (setup dips correctly!)
Konami K056230 makes 'GTI Club', 'Jet Wave', 'Midnight Run', 'Polygonet Commanders', 'Poly-Net Warriors' and 'Winding Heat' playable in LAN.
Namco C139 makes 'Driver's Eyes' working other C139 games are working but the network is highly timing critical (unstable).
all comms now use ASIO for async (non-blocking) operation.
--- image spam below ---
Konami K056230
Namco C139
pdriftl - https://www.youtube.com/watch?v=R8OlbqSLZhQ smgp - https://www.youtube.com/watch?v=0uVJrce4-es midnrun / windheat - https://www.youtube.com/watch?v=0LWOftPXWNE gticlub - https://www.youtube.com/watch?v=Jmj_X1fdC2o ridgera2 - https://www.youtube.com/watch?v=tid2uVlX68k raverace - https://www.youtube.com/watch?v=nx0NL4GTbWk
All or nearly all of my feedback points have been addressed, it doesn't appear that I can mark them as resolved on my own, though.
All or nearly all of my feedback points have been addressed, it doesn't appear that I can mark them as resolved on my own, though.
To be honest I can't even see any of those on github. From my point of view, your comment was the first interaction on that topic. github sometimes seems weird.
I'm okaying decision to remove "NODEVICE_LAN" flag from thunderh/thunderhu because of its LANC usage for something other than cabinet linking.
All or nearly all of my feedback points have been addressed, it doesn't appear that I can mark them as resolved on my own, though.
Did you remember to hit the “complete review” button or whatever it’s called?
Sorry, almost 3AM, too tired to look at the full diff again tonight. I’ll get back to it some time tomorrow.
All or nearly all of my feedback points have been addressed, it doesn't appear that I can mark them as resolved on my own, though.
Did you remember to hit the “complete review” button or whatever it’s called?
I don't even see a "Complete Review" button, which is bizarre as I certainly seemed to be able to open one.
While I see your points, I have to say I am getting really frustrated.
YES, there are some possibilities to "freeze" MAME momentarily.
That being said however... The code for listening and connecting is there by design. The idea being that the "other" instance might not be started yet, hence multiple tries - or the the user is reseting via F3.
Also, the listening socket stops listening after accepting one connection. In case the active connection gets terminated for some reason, it simply gets ready to accept a new one.
The read loop has been there (at least in my code) since the beginning, as fragmented packets should not be happening in the first place. The newly added write-loop got there, because you requested me to handle partial writes that "might" appear.
Both loops will instantly exit as soon as the socket returns any error (besides EWOULDBLOCK on partial reads)
The code is blocking by design, there even is - at least on some comm implementations - an option implemented to "framesync" the emulation. this is done by deliberately spinning until a special frame has been received or an error occured.
I know that my code probably is not be the "best" way of doing things. But it is all that I have for now - and it is working. I'm not a professinal C++ programmer, heck I don't even use C++ for my personal stuff.
I buy boards, "borrow" chips and reverse engineer them by building hardware contraptions to hook them up to an arduino or an PC via ISA-bus. Right now I am waiting for a pcb to arrive so I can hook up a real C139.
I spend ages single stepping through code that reads/writes to a chip and try to figure out what the code is expecting and how everything is supposed to be working. Then I come up with some emulation, and refine that, until it is "working".
The first code for that (m1comm) has been that way almost unchanged since I came up with it in 2012. I was the one responsible for "socket"-files to be able to listen in the first place.
Most of that stuff has been running for several years now on my cabinets, as seen on my youtube channel.
I'll happyly add a "TODO: make stuff non blocking" in there, but most certainly will not rewrite the core logic of it right now.
--
As we are talking about user experience... what is the typical use case?
The normal user won't use the COMM stuff in the first place. So the default of connecting to itself (via 127.0.0.1) should not be an issue.
The advanced user that wants to use the COMM stuff to link up several instances will probably do that on the local machine, or on the local network. That same user likely will know that configuration or connection issues will cause trouble. Some people even did Virtua Racing sessions over the internet without issues.
-- EDIT -- Right now, you start up 4 instances of MAME for 4 player "Midnight Run" action.
Assuming the remote machines stay "on"... If one instance gets "closed", the last instance in the ring will enter a loop of:
- "freeze" for 1 second (while trying to reconnect)
- react to user input (like holdin ESC)
unpleasent, yes - but no need to fire up task manager and "kill" mame.
If the other machine simply turns off, hence not closing the socket gracefully, the last instance will freeze for as long as the timeout for TCP sockets is set. we could set SO_RCVTIMEO, SO_SNDTIMEO and/or SO_KEEPALIVE to "limit" the issue. this won't fix the connect timeout though.
Like I previously mentioned, linking multiple mame instances is a rather advanced topic in the first place.
The ASIO stuff in cps2comm seems simple enough. I have no issue in rewriting things - at a later point, that is.
I want to throw my two cents in on this whole "OSD vs. ASIO" thing: Given the fact that @SailorSat here isn't just the person developing these networking-board implementations, but is the person actively using them, she knows better than anyone whether @cuavas's assertions of doom and gloom about main-thread blocking are likely to happen given the workflow of a typical user of this feature.
This is a feature intended not for playing these games across the Internet, but across multiple instances of MAME on either the same machine or on a LAN, for local parties and such.
The concerns being expressed about "what happens if a connection drops out" are broadly irrelevant, because the feature already self-selects for users that know what they're doing. Average users are almost certainly going to try to use it across the Internet, and they'll end up disappointed no matter what Ariane here does - because a user trying to do that is one of those "Doctor, it hurts when I hit myself with this hammer" situations.
The more likely scenario is temporary stutter or blocking of the main thread, if it can be blocked at all. When MAME already becomes unusable on Windows if the user erroneously fullscreens MAME while running with -debug and hits a breakpoint - which results in a locked-up MAME that's almost impossible to even get Task Manager to kill due to the fullscreen output taking priority over the desktop window manager - there's no value or point to focusing on the molehill that already has a mountain next to it.
Stop holding up contributors' progress in other areas of MAME just because there are some sins in the core that could and should be rectified. With the nebulous-to-nonexistent timeline for addressing those issues, it comes off as hypocrisy given the history of giving grief to @smf- for purportedly doing the same.
@cuavas Do you have any objections to this besides theoretical ones about MAME hanging? Because I'm gonna merge it if not, and then SailorSat will have a solid base to rework it for ASIO.
@cuavas Do you have any objections to this besides theoretical ones about MAME hanging? Because I'm gonna merge it if not, and then SailorSat will have a solid base to rework it for ASIO.
It isn't theoretical. Having MAME hang because some other random program is listening on a particular TCP port, or having it lock up if it loses connectivity is not an acceptable use experience.
I'm going to revert it if it's merged in this state, because if it is merged like this, it's inevitably going to become one more thing I have to fix myself. (But in this case, it's lots more things to fix.)
If you're feeling under pressure because Haze announced that this will be in the next release, meeting it in a state like this isn't a solution.
People would be opting in to running the game linked, at which point there is a whole universe of things out of our control. And I'm fine with that. I'll add the "you are now on your own" popmessage myself if it helps. SailorSat's previous networking stuff has many of the same theoretical issues and has been in mainline for years without complaints, so this just feels malicious.
I'm with RB on this one. With all of your criticisms for smf, this just makes you look like a massive hypocrite for doing exactly the same thing, Vas.
"It's inevitably going to become one more thing I have to fix", get down off your cross, dude. Ariane has already said she's willing to look into a rewrite with enough time, so no, it isn't.
You perpetually bemoan things not being done to your liking, yet you never actually document what "to your liking" entails.
You perpetually go on about how things need to be better, yet you never document what "better" actually is.
Let's not forget that ultimately the current coding guidelines that exist on https://docs.mamedev.org/ weren't written by you, they were written by me, out of frustration for your perpetual lamentations about how nobody could get the coding style right.
At this point it seems like you genuinely enjoy being able to portray yourself as this set-upon victim of circumstance. Quit your bullshit.
So I tried some asio rewrite for m1comm ( see sega.zip )
However I run into some compiler error. Not sure if I did something wrong.
GCC 11.2.0 detected Compiling src/mame/sega/model1.cpp... In file included from ../../../../../3rdparty/softfloat/milieu.h:36, from ../../../../../src/devices/cpu/i386/i386.h:11, from ../../../../../src/mame/sega/model1.cpp:598: ../../../../../3rdparty/softfloat/mamesf.h:5: error: "LITTLEENDIAN" redefined [-Werror] 5 | #define LITTLEENDIAN | In file included from ../../../../../3rdparty/asio/include/asio/detail/socket_types.hpp:33, from ../../../../../3rdparty/asio/include/asio/detail/win_tss_ptr.hpp:23, from ../../../../../3rdparty/asio/include/asio/detail/tss_ptr.hpp:25, from ../../../../../3rdparty/asio/include/asio/detail/call_stack.hpp:20, from ../../../../../3rdparty/asio/include/asio/detail/thread_context.hpp:20, from ../../../../../3rdparty/asio/include/asio/detail/recycling_allocator.hpp:20, from ../../../../../3rdparty/asio/include/asio/detail/handler_alloc_helpers.hpp:21, from ../../../../../3rdparty/asio/include/asio/detail/executor_function.hpp:19, from ../../../../../3rdparty/asio/include/asio/execution/any_executor.hpp:24, from ../../../../../3rdparty/asio/include/asio/execution.hpp:19, from ../../../../../3rdparty/asio/include/asio/any_completion_executor.hpp:22, from ../../../../../3rdparty/asio/include/asio.hpp:18, from ../../../../../src/osd/asio.h:28, from ../../../../../src/mame/sega/m1comm.h:14, from ../../../../../src/mame/sega/model1.h:9, from ../../../../../src/mame/sega/model1.cpp:596: D:/msys64/mingw64/x86_64-w64-mingw32/include/winsock2.h:511: note: this is the location of the previous definition 511 | #define LITTLEENDIAN 0x0001 | cc1plus.exe: all warnings being treated as errors
It appears I cannot use softfloat and asio at the same time right now?
the basic idea of operation of my rewrite:
- made connect and accept "async".
- use a timer to keep track of connection timeouts.
- keep track of the sockets status in a simple variable (0 disconnected / 1 connecting or waiting for accept / 2 connected)
- as long as either of the sockets is in status 1, poll the io_context to process any async event that might have happened (without blocking).
- before "reading", check if enough data is in the buffer using "available"
- tread a partial write as "error" and disconnect.
- (not in the provided code) set TCP_NO_DELAY for the outgoing socket to keep latency down.
that way the code cannot block. partial writes should never happen, as long as the connection is healthy.
possible ideas what to add:
- do actual (async) dnsresolves for comm_localhost / comm_remotehost.
- setup larger buffers on the sockets - probably 64kB should be fine.
So... I did successfully rewrite some of the comm implementations to ASIO.
However ASIO seems to clash with both the SHARC DRC as well as the M68000 code. I won't even try mb89372 and mb89374 for now...
Model2 fails m2comm.txt
Sega Y-Board, Namco C139 and Konami k056230 fail k056230.txt namco_c139.txt ybdcomm.txt
And those drivers compile fine if you don't include asio.h? Which would be strange, since the errors you posted look like GCC 11.2 bugs or the version is outdated.
And those drivers compile fine if you don't include asio.h? Which would be strange, since the errors you posted look like GCC 11.2 bugs or the version is outdated.
it does compile fine with the "old" code that uses osdfile.h rather than asio.h
EDIT I will update my buildtools, just to make sure thats not the issue.
EDIT 2 Still the same issue with up2date build tools.
Converted all my comm implementations to ASIO. Thanks to @happppp for getting those compile issues solved.
My ASIO based comms do:
- actually "resolve" localhost/port and remotehost/port on startup once.
- connect and accept "async".
- use a timer to keep track of connection timeouts.
- keep track of the sockets status in a simple variable (0 disconnected / 1 connecting or waiting for accept / 2 connected)
- as long as either of the sockets is in status 1, poll the io_context to process any async event that might have happened (without blocking).
- before "reading", check if enough data is in the buffer by "peeking"
- tread a partial write as "error" and disconnect.
- set TCP_NO_DELAY for the outgoing socket to keep latency down.
- setup largert buffers on sockets - 64k for TX, 512k for RX (should be more than enough)
that way the code cannot block. partial writes should never happen, as long as the connection is healthy.
@cuavas : I hope this is fine now.
@cuavas Do you have any objections to this besides theoretical ones about MAME hanging? Because I'm gonna merge it if not, and then SailorSat will have a solid base to rework it for ASIO.
It isn't theoretical. Having MAME hang because some other random program is listening on a particular TCP port, or having it lock up if it loses connectivity is not an acceptable use experience.
I'm going to revert it if it's merged in this state, because if it is merged like this, it's inevitably going to become one more thing I have to fix myself. (But in this case, it's lots more things to fix.)
If you're feeling under pressure because Haze announced that this will be in the next release, meeting it in a state like this isn't a solution.
This is more of your authoritarian nonsense that I find so rude. "Rule by fear" in this case the threat of revert. For 99% of people this would have been fine, and SailorSat would have made the changes either before or after. You're moaning about something that the people using it have been perfectly happy with for other systems for years.
Also I'm not 'putting pressure on' SailorSat by covering this, I'm covering cool stuff that happens in MAME. I can just not bother instead, and people will go back to thinking nothing of note has happened in MAME for years. You're draining all the fun out of this like a vampire.
Any reasonable project management would have just merged it and let the kinks be ironed out, not continued to load more and more demands on a PR by moving goalposts. It followed how things were being done in MAME, it worked, it could be built upon. It only becomes your problem to fix when you piss the people working on stuff so much they don't actually come back, which you've done a few times to say the least, even if most of those people haven't said a word and instead silently walked away. (and no, this isn't just me saying that, I was informed by Hammy recently that somebody else has recently made the same decision in another area of promise where you've moved the goalposts repeatedly and come across as rude about what they've been spending several years on)
I find overloading PRs with requests like this to be the same as when we do something cool, and then all the users say 'but what about x instead' I believe you've said you find that rude yourself, or at least many other devs have. Appreciate what has been done, the value of a PR, rather than focusing on what hasn't.
We have similar code in supermodel for networking to sailorsat's original code and no one has ever complained. It's never been an issue.
Funny how when SailorSat makes the requested changes, Cuavas goes radio silent. What gives?
Funny how when SailorSat makes the requested changes, Cuavas goes radio silent. What gives?
It's the Thursday before code-freeze weekend release. It's not that deep, and you're not as smart as you believe you are.
Funny how when SailorSat makes the requested changes, Cuavas goes radio silent. What gives?
It's the Thursday before code-freeze weekend release. It's not that deep, and you're not as smart as you believe you are.
Fair enough, I got too passionate about this thread and left a stupid comment. My apologies. That said, I still agree with you and MameHaze on this issue.
So... almost 3 weeks of radio silence now?
@cuavas is it still in a state where you'd revert it if someone else merged it?
@SailorSat to properly initialize (zerofill) device class variables, we don't do it in the .h file, but in the .cpp file. Either in the constructor or at device_start.
So, a method like: u8 m_myvar = 0; u8 m_myarray[16]{};
is fine in a MAME driver file, but not in a device header file that is often included multiple times by drivers.
Anyone else have feedback? If no, let's merge it 24hrs from now.
@SailorSat BTW could you update your 1st post? Mention ASIO? It'll be the commit msg (whatsnew.txt)
Also, I'll make sure to ask: is everything working as you expect it to after the ASIO changes?