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

BBA: Tap-less/Built-in network translation #10564

Closed
wants to merge 28 commits into from
Closed

BBA: Tap-less/Built-in network translation #10564

wants to merge 28 commits into from

Conversation

schthack
Copy link
Contributor

@schthack schthack commented Apr 7, 2022

Added a new network option for gamecube, no Tap needed.
Also fixed a bug in the BBA code where the buffer end 1 early.

This is a WIP and has been tested on PSO GC for now. Build tested on windows and Linux.

@Pokechu22 Pokechu22 marked this pull request as draft April 7, 2022 01:24
@Pokechu22
Copy link
Contributor

Can you please rebase this PR to fix the merge conflict in Source/Core/DolphinLib.vcxproj? That way I can run Dolphin's CI on it.

@Pokechu22
Copy link
Contributor

FYI, you can click "view all 115 lines" to see the whole list of lint errors (the default view only shows the last 30 lines).

@schthack
Copy link
Contributor Author

schthack commented Apr 7, 2022

Yes thx! just missed a few doing them by hand

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

I gave it a first look. You might want to take inspiration from Common/Network.h and Core/NetworkCaptureLogger for some of the packet construction facilities we have.

You might want to have a look at Core/IOS/Network/Socket for the socket handling if relevant (TCP/UDP socket and the socket table). I don't known how tight to SFML's socket the code is but I'm sure with a more object oriented approach many if-else can be removed using a class.

// i18n: MAC stands for Media Access Control. A MAC address uniquely identifies a network
// interface (physical) like a serial number. "MAC" should be kept in translations.
address_label = new QLabel(tr("Enter the DNS server to use:"));
address_placeholder = QString::fromStdString("8.8.8.8");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't QStringLiteral("8.8.8.8") be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mabe, i honestly just did like the 2 other one (tap and xlink)

i will take a look at those network stuff

Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.h Show resolved Hide resolved
Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h Show resolved Hide resolved
Source/Core/Core/HW/EXI/EXI_Device.h Outdated Show resolved Hide resolved
Source/Core/DolphinLib.vcxproj Outdated Show resolved Hide resolved
Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Some more comments regarding the coding style.

Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
@JMC47
Copy link
Contributor

JMC47 commented Apr 16, 2022

Does this work for games like MK:DD, Kirby Air Ride and 1080 Avalanche, which don't use a server?

@schthack
Copy link
Contributor Author

@JMC47 Doing the fine tuning for them, got all of them working on my setup, doing some more testing with other ppl

@ShiftaDeband
Copy link
Contributor

ShiftaDeband commented Apr 16, 2022

Confirmed to be working on macOS x86/x64 and ARM.

I had to change the line referenced in the build fail (Builtin.cpp, line 502) slightly to get it to compile.

Screen Shot 2022-04-15 at 10 45 24 PM

Screen Shot 2022-04-16 at 12 19 43 AM

@nolrinale
Copy link
Contributor

nolrinale commented Apr 16, 2022

Following @ShiftaDeband I commented the line number 502 ref->from.sin_addr.S_un.S_addr = htonl(ref->target.toInteger()); inside BuiltIn.cpp and it let me compile on macOS BigSur.

Testing with PSO Episode 3 lobby loaded at the correct speeds
image

Specs:
Big Sur 11.6.5
Macbook Air M1 16GB / 8 GPU

Wonderful work you guys are doing!

Copy link
Contributor

@nolrinale nolrinale left a comment

Choose a reason for hiding this comment

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

Tested a last time on macOS and it seems very stable. But I cannot find the new setting to setup the IP as the commit comment says

@schthack
Copy link
Contributor Author

Tested a last time on macOS and it seems very stable. But I cannot find the new setting to setup the IP as the commit comment says

BBA_BUILTIN_IP <- i made it a ini config only, set it to an ip to force it, or remove to have it auto.

This was needed for LAN play, games send theire IP in various packets and it work better if the IP being given to the gamecube is the same as your pc/mac/host

For server game like PSO and Homeland any ip should work. The option is there for MK DD, Air ride and 1080.

Those game also relay on Simple discovery service(port 1900) wich is also used by windows and probably macos. On windows stoping the SSDPSRV service is needed for game to find each others

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Those game also relay on Simple discovery service(port 1900) wich is also used by windows and probably macos. On windows stoping the SSDPSRV service is needed for game to find each others

If that behaviour is detected, you should probably add a dedicated log message or popup to warn users about it.

Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Another thing I missed, since you use pointers to raw memory and memcpy a lot, you should add a static assert in all the structures in Network.h and BuiltIn.h ensuring that these are trivially copiable.

Also, C++ doesn't guarantee memcpy to be available in the global namespace so you should prefix them with the std:: namespace.

When possible, you should also prefer using std::fill and std::fill_n rather than std::memset.

Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
@Rumi-Larry
Copy link

Recommended PR title: "BBA: Tap-less/Built-in network translation"

@schthack schthack changed the title Tap Less / built in network translation BBA: Tap-less/Built-in network translation May 10, 2022
@schthack
Copy link
Contributor Author

schthack commented May 13, 2022

@sepalani can you run the build so i can see the lint errors! Also all major issue have been resolved so this would be ready to review and eventualy merge

@shuffle2
Copy link
Contributor

fyi: you will need to rebase it on current master to make windows build work (sorry)

@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • ab11-homebrew on ogl-lin-radeon: diff
  • aeon-charge-attack on ogl-lin-radeon: diff
  • bk-tev on ogl-lin-radeon: diff
  • burnout2-vehicletextures on ogl-lin-radeon: diff
  • chibi-robo-fastdepth on ogl-lin-radeon: diff
  • chibi-robo-zfighting on ogl-lin-radeon: diff
  • custom-brawl-char on ogl-lin-radeon: diff
  • dbz-depth on ogl-lin-radeon: diff
  • djfny-menu on ogl-lin-radeon: diff
  • djhero2-blend on ogl-lin-radeon: diff
  • DKCR-Char on ogl-lin-radeon: diff
  • DKCR-fast-depth on ogl-lin-radeon: diff
  • ea-pink on ogl-lin-radeon: diff
  • ed-updated on ogl-lin-radeon: diff
  • et-vid on ogl-lin-radeon: diff
  • find-mii on ogl-lin-radeon: diff
  • fishing-resort-map on ogl-lin-radeon: diff
  • fog-adj on ogl-lin-radeon: diff
  • fortune-street on ogl-lin-radeon: diff
  • fortune-street-fog on ogl-lin-radeon: diff
  • fortune-street-white-box on ogl-lin-radeon: diff
  • fsa-layers on ogl-lin-radeon: diff
  • f-zero-rain on ogl-lin-radeon: diff
  • goldeneye-depth on ogl-lin-radeon: diff
  • hb-discgolf on ogl-lin-radeon: diff
  • inverted-depth-range on ogl-lin-radeon: diff
  • jb-shadow on ogl-lin-radeon: diff
  • jd2-fmv on ogl-lin-radeon: diff
  • jj-awae-mirrored on ogl-lin-radeon: diff
  • kirby-logicop on ogl-lin-radeon: diff
  • kirby-shadows on ogl-lin-radeon: diff
  • last-story-shadows on ogl-lin-radeon: diff
  • lego-star-wars-crane-shadow on ogl-lin-radeon: diff
  • lesson08 on ogl-lin-radeon: diff
  • line-width-test on ogl-lin-radeon: diff
  • lm-mario-portrait on ogl-lin-radeon: diff
  • luigi-shadows on ogl-lin-radeon: diff
  • major-minor on ogl-lin-radeon: diff
  • mario-baseball-shadows on ogl-lin-radeon: diff
  • mario-golf-oob on ogl-lin-radeon: diff
  • mario-sluggers-bar on ogl-lin-radeon: diff
  • mario-tennis-menu on ogl-lin-radeon: diff
  • MaS-LOG-wiimote on ogl-lin-radeon: diff
  • megaman-heat on ogl-lin-radeon: diff
  • melee-depth on ogl-lin-radeon: diff
  • melee-lighting on ogl-lin-radeon: diff
  • metroid-visor on ogl-lin-radeon: diff
  • mii-channel on ogl-lin-radeon: diff
  • milotic-texture on ogl-lin-radeon: diff
  • mini-ninjas on ogl-lin-radeon: diff
  • mkdd-babypark on ogl-lin-radeon: diff
  • mkdd-efb on ogl-lin-radeon: diff
  • mkw-bridge on ogl-lin-radeon: diff
  • mkw-flags on ogl-lin-radeon: diff
  • mkwii-bluebox on ogl-lin-radeon: diff
  • monkeyball-fuse on ogl-lin-radeon: diff
  • mp2-scanner on ogl-lin-radeon: diff
  • mp3-bloom on ogl-lin-radeon: diff
  • mp4-vertexcache on ogl-lin-radeon: diff
  • mp7-text on ogl-lin-radeon: diff
  • mp8-widescreen on ogl-lin-radeon: diff
  • mtennis-zfreeze on ogl-lin-radeon: diff
  • my-word-coach on ogl-lin-radeon: diff
  • nddemo-bumpmapping on ogl-lin-radeon: diff
  • nddemo-lighting on ogl-lin-radeon: diff
  • nfsu-purplerect on ogl-lin-radeon: diff
  • nfsu-reflections on ogl-lin-radeon: diff
  • nhl-slap on ogl-lin-radeon: diff
  • nintendo-channel on ogl-lin-radeon: diff
  • nsmbw-coins on ogl-lin-radeon: diff
  • nsmbw-intro on ogl-lin-radeon: diff
  • pbr-sfx on ogl-lin-radeon: diff
  • pm-hc-jp on ogl-lin-radeon: diff
  • pw-black-bars on ogl-lin-radeon: diff
  • rs2-bumpmapping on ogl-lin-radeon: diff
  • rs2-glass on ogl-lin-radeon: diff
  • rs2-skybox on ogl-lin-radeon: diff
  • rs2-zfreeze on ogl-lin-radeon: diff
  • rs3-bumpmapping on ogl-lin-radeon: diff
  • rs3-skybox2 on ogl-lin-radeon: diff
  • sadx-ui on ogl-lin-radeon: diff
  • sfa-shadows on ogl-lin-radeon: diff
  • sf-assault-flashing on ogl-lin-radeon: diff
  • shadow-eyes on ogl-lin-radeon: diff
  • simpsons-game on ogl-lin-radeon: diff
  • smb-mirror on ogl-lin-radeon: diff
  • smg2-fog on ogl-lin-radeon: diff
  • smg-marioeyes on ogl-lin-radeon: diff
  • smg-mmg on ogl-lin-radeon: diff
  • smg-roar on ogl-lin-radeon: diff
  • sms-bubbles on ogl-lin-radeon: diff
  • sms-gc on ogl-lin-radeon: diff
  • sms-water on ogl-lin-radeon: diff
  • soa-black on ogl-lin-radeon: diff
  • soniccolors-mm on ogl-lin-radeon: diff
  • sonic-riders-blur on ogl-lin-radeon: diff
  • sonic-riders-zg-4p on ogl-lin-radeon: diff
  • sonicriderszg-gb on ogl-lin-radeon: diff
  • spyro-bloom on ogl-lin-radeon: diff
  • spyro-depth on ogl-lin-radeon: diff
  • ssbb-mod-lloyd on ogl-lin-radeon: diff
  • ssbm-pointsize on ogl-lin-radeon: diff
  • ss-map on ogl-lin-radeon: diff
  • super-sluggers-white-out on ogl-lin-radeon: diff
  • sw3-dt on ogl-lin-radeon: diff
  • thps3-earlyz on ogl-lin-radeon: diff
  • thps4-shadow on ogl-lin-radeon: diff
  • tla-menu on ogl-lin-radeon: diff
  • tos-invis-char on ogl-lin-radeon: diff
  • tp-skin on ogl-lin-radeon: diff
  • tsp3-pinkgrass on ogl-lin-radeon: diff
  • vegas-party-depth on ogl-lin-radeon: diff
  • viewitful-joe-distortion on ogl-lin-radeon: diff
  • xenoblade-menu on ogl-lin-radeon: diff
  • ztp-grass on ogl-lin-radeon: diff
  • zww-armos on ogl-lin-radeon: diff
  • zww-water on ogl-lin-radeon: diff
  • zww-waves on ogl-lin-radeon: diff
  • ab11-homebrew on uberogl-lin-radeon: diff
  • aeon-charge-attack on uberogl-lin-radeon: diff
  • bk-tev on uberogl-lin-radeon: diff
  • burnout2-vehicletextures on uberogl-lin-radeon: diff
  • chibi-robo-fastdepth on uberogl-lin-radeon: diff
  • chibi-robo-zfighting on uberogl-lin-radeon: diff
  • custom-brawl-char on uberogl-lin-radeon: diff
  • dbz-depth on uberogl-lin-radeon: diff
  • djfny-menu on uberogl-lin-radeon: diff
  • djhero2-blend on uberogl-lin-radeon: diff
  • DKCR-Char on uberogl-lin-radeon: diff
  • DKCR-fast-depth on uberogl-lin-radeon: diff
  • ea-pink on uberogl-lin-radeon: diff
  • ed-updated on uberogl-lin-radeon: diff
  • et-vid on uberogl-lin-radeon: diff
  • find-mii on uberogl-lin-radeon: diff
  • fishing-resort-map on uberogl-lin-radeon: diff
  • fog-adj on uberogl-lin-radeon: diff
  • fortune-street on uberogl-lin-radeon: diff
  • fortune-street-fog on uberogl-lin-radeon: diff
  • fortune-street-white-box on uberogl-lin-radeon: diff
  • fsa-layers on uberogl-lin-radeon: diff
  • f-zero-rain on uberogl-lin-radeon: diff
  • goldeneye-depth on uberogl-lin-radeon: diff
  • hb-discgolf on uberogl-lin-radeon: diff
  • inverted-depth-range on uberogl-lin-radeon: diff
  • jb-shadow on uberogl-lin-radeon: diff
  • jd2-fmv on uberogl-lin-radeon: diff
  • jj-awae-mirrored on uberogl-lin-radeon: diff
  • kirby-logicop on uberogl-lin-radeon: diff
  • kirby-shadows on uberogl-lin-radeon: diff
  • last-story-shadows on uberogl-lin-radeon: diff
  • lego-star-wars-crane-shadow on uberogl-lin-radeon: diff
  • lesson08 on uberogl-lin-radeon: diff
  • line-width-test on uberogl-lin-radeon: diff
  • lm-mario-portrait on uberogl-lin-radeon: diff
  • luigi-shadows on uberogl-lin-radeon: diff
  • major-minor on uberogl-lin-radeon: diff
  • mario-baseball-shadows on uberogl-lin-radeon: diff
  • mario-golf-oob on uberogl-lin-radeon: diff
  • mario-sluggers-bar on uberogl-lin-radeon: diff
  • mario-tennis-menu on uberogl-lin-radeon: diff
  • MaS-LOG-wiimote on uberogl-lin-radeon: diff
  • megaman-heat on uberogl-lin-radeon: diff
  • melee-depth on uberogl-lin-radeon: diff
  • melee-lighting on uberogl-lin-radeon: diff
  • metroid-visor on uberogl-lin-radeon: diff
  • mii-channel on uberogl-lin-radeon: diff
  • milotic-texture on uberogl-lin-radeon: diff
  • mini-ninjas on uberogl-lin-radeon: diff
  • mkdd-babypark on uberogl-lin-radeon: diff
  • mkdd-efb on uberogl-lin-radeon: diff
  • mkw-bridge on uberogl-lin-radeon: diff
  • mkw-flags on uberogl-lin-radeon: diff
  • mkwii-bluebox on uberogl-lin-radeon: diff
  • monkeyball-fuse on uberogl-lin-radeon: diff
  • mp2-scanner on uberogl-lin-radeon: diff
  • mp3-bloom on uberogl-lin-radeon: diff
  • mp4-vertexcache on uberogl-lin-radeon: diff
  • mp7-text on uberogl-lin-radeon: diff
  • mp8-widescreen on uberogl-lin-radeon: diff
  • mtennis-zfreeze on uberogl-lin-radeon: diff
  • my-word-coach on uberogl-lin-radeon: diff
  • nddemo-bumpmapping on uberogl-lin-radeon: diff
  • nddemo-lighting on uberogl-lin-radeon: diff
  • nfsu-purplerect on uberogl-lin-radeon: diff
  • nfsu-reflections on uberogl-lin-radeon: diff
  • nhl-slap on uberogl-lin-radeon: diff
  • nintendo-channel on uberogl-lin-radeon: diff
  • nsmbw-coins on uberogl-lin-radeon: diff
  • nsmbw-intro on uberogl-lin-radeon: diff
  • pbr-sfx on uberogl-lin-radeon: diff
  • pm-hc-jp on uberogl-lin-radeon: diff
  • pw-black-bars on uberogl-lin-radeon: diff
  • rs2-bumpmapping on uberogl-lin-radeon: diff
  • rs2-glass on uberogl-lin-radeon: diff
  • rs2-skybox on uberogl-lin-radeon: diff
  • rs2-zfreeze on uberogl-lin-radeon: diff
  • rs3-bumpmapping on uberogl-lin-radeon: diff
  • rs3-skybox2 on uberogl-lin-radeon: diff
  • sadx-ui on uberogl-lin-radeon: diff
  • sfa-shadows on uberogl-lin-radeon: diff
  • sf-assault-flashing on uberogl-lin-radeon: diff
  • shadow-eyes on uberogl-lin-radeon: diff
  • simpsons-game on uberogl-lin-radeon: diff
  • smb-mirror on uberogl-lin-radeon: diff
  • smg2-fog on uberogl-lin-radeon: diff
  • smg-marioeyes on uberogl-lin-radeon: diff
  • smg-mmg on uberogl-lin-radeon: diff
  • smg-roar on uberogl-lin-radeon: diff
  • sms-bubbles on uberogl-lin-radeon: diff
  • sms-gc on uberogl-lin-radeon: diff
  • sms-water on uberogl-lin-radeon: diff
  • soa-black on uberogl-lin-radeon: diff
  • soniccolors-mm on uberogl-lin-radeon: diff
  • sonic-riders-blur on uberogl-lin-radeon: diff
  • sonic-riders-zg-4p on uberogl-lin-radeon: diff
  • sonicriderszg-gb on uberogl-lin-radeon: diff
  • spyro-bloom on uberogl-lin-radeon: diff
  • spyro-depth on uberogl-lin-radeon: diff
  • ssbb-mod-lloyd on uberogl-lin-radeon: diff
  • ssbm-pointsize on uberogl-lin-radeon: diff
  • ss-map on uberogl-lin-radeon: diff
  • super-sluggers-white-out on uberogl-lin-radeon: diff
  • sw3-dt on uberogl-lin-radeon: diff
  • thps3-earlyz on uberogl-lin-radeon: diff
  • thps4-shadow on uberogl-lin-radeon: diff
  • tla-menu on uberogl-lin-radeon: diff
  • tos-invis-char on uberogl-lin-radeon: diff
  • tp-skin on uberogl-lin-radeon: diff
  • tsp3-pinkgrass on uberogl-lin-radeon: diff
  • vegas-party-depth on uberogl-lin-radeon: diff
  • viewitful-joe-distortion on uberogl-lin-radeon: diff
  • xenoblade-menu on uberogl-lin-radeon: diff
  • ztp-grass on uberogl-lin-radeon: diff
  • zww-armos on uberogl-lin-radeon: diff
  • zww-water on uberogl-lin-radeon: diff
  • zww-waves on uberogl-lin-radeon: diff

automated-fifoci-reporter

@schthack schthack marked this pull request as ready for review May 14, 2022 00:19
Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Some more comments regarding the code.

What would be nice to change if that doesn't impact the performance and the behaviour would be replacing the C-style approach with their C++ equivalent:

  • static array -> std::array
  • allocated array -> std::vector
  • C style cast -> C++ cast

Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Common/Network.h Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp Outdated Show resolved Hide resolved
@AdmiralCurtiss
Copy link
Contributor

Can you rebase this on master and clean up the commit history? Just squashing to 1 commit is fine if there's no meaningful intermediate state.

some vector fix
@schthack
Copy link
Contributor Author

I messed up by fetching instead of a rebase, i cannot clean most of the commit. i will close this pr and make a new one with a single commit

@schthack schthack closed this May 22, 2022
@Tilka
Copy link
Member

Tilka commented May 22, 2022

You don't need to close the PR, you can just redo things locally and force-push to the same remote branch name. If you want to, you can even reopen this PR.

@sepalani
Copy link
Contributor

You can also use git reflog to revert to almost any of the previous states your repository had.

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