Hiding and not implementing the copy constructor is a pre-C++11 thing.
It should also be noted that a copy constructor, as defined by the
language, contains a const qualifier on its parameter, so this wouldn't
have prevented copies from being performed.
Now that everything has been changed to use the new structs, the old
methods and structs can be removed.
And while I was changing the base device class, I also moved the
"unsupported command" code to a separate function. It was pretty silly
to copy the same 3 lines for ~5 commands.
This adds well-defined structs that are responsible for parsing
resource requests, instead of duplicating the logic and offsets all
over IOS HLE. Command handler functions are now passed parsed requests
instead of a command address.
This may not seem like a very important change, but it removes the
need to remember all of the struct offsets or copy/paste existing
struct request variables. It also prevents nasty bugs which have
occurred in the past, such as parsing an ioctl as if it were an ioctlv
(that's way too easy to do if you pass command addresses directly);
or writing something to 0x0, which can easily happen by mistake with
a close handler that can be called with invalid command addresses.
Bonus changes:
- The return code is not an obscure Memory::Write_U32 anymore, but an
explicit, more obvious SetReturnValue() call. (Which correctly takes
a s32 instead of a u32, since return codes are signed.)
- Open handlers are now only responsible for returning an IOS ret code,
and Close handlers don't return anything and don't have to worry
about checking whether the request is a real one anymore.
- DumpAsync was moved to the ioctlv request struct, because it did
not really make sense to make it part of the IOS device and it only
works for ioctlvs.
All current usages have been removed; they will be readded in a
later commit.
As of this commit, nothing uses the structs yet. Usages will be
migrated progressively.
Hiding and not implementing the copy constructor is a pre-C++11 thing.
It should also be noted that a copy constructor, as defined by the language,
contains a const qualifier on its parameter, so this wouldn't have
prevented copies from being performed.
It also follows that if the copy constructor is deleted, then copy
assignment should also be forbidden.
wxChoice controls don't display any titles.
By the way, why is the file called DebuggerPanel.cpp
even though it implements the Video debug panel specifically?
The current implementations do many things wrong but work well enough to
run the Dragon Quest X installer until the very end. The game itself
crashes when being launched from its System Menu channel unfortunately
so it is hard to verify whether the install properly worked or not.
There are plenty of "TODO(wfs)" sprinkled around this codebase with
things that are knowingly done wrong. The most important one right now
is that content extraction is done by buffering everything into memory
instead of properly streaming the data to disk (and processing
asynchronously), which causes freezes. It is likely to not cause any
practical issues since only the installer and the updater should use
this anyway.
Without this, attempts to savestate std::set will fail with an error
about dropping the const qualifier.
<Lioncash> leoetlino: I'll try to break it down: So, when you do a
ranged-for on a container, it's essentially syntactic sugar over begin
and end iterators. std::set is an associative container where the key
type is the same as the value type, and so it's required that all
iterator functions return constant iterators. If this wasn't a
requirement, it would allow changing the ordering of elements from
outside of the set's API (this is bad).
This attempts to make some bit arithmetic more self-documenting and also
make it easier during review to identify potential off-by-one errors by
making it possible to just specify which bits are being extracted.
Functions both support the case where bits being extracted can vary and
fixed bit extraction. In the case the bits are fixed, compile-time asserts
are present to prevent accidental API usage at compile-time.
e.g. Instead of shifting and masking to get bits 10 to 15,
Common::ExtractBits<10, 15>(value) can just be done instead.
There are several things wrong with this implementation. The first being
that since we still don't have a proper ticket/tmd handling library, we
hardcode offsets once again to fetch TMD fields. The second being that
we don't stream data to the disk and we buffer everything in memory. The
third being that we don't properly fetch the content index for
decryption, which is prone to breaking.
But hey, it works well enough to install the DQX channel!
This library implements basic parsing support for some of the IOS ES
formats we need to extract data from. Currently only implements TMD
functions, but some ticket handling functions from DiscIO should likely
be moved here in the future.
These two functions load either a signed ticket or a raw ticket from the
emulated NAND.
The ticket signature skip is refactored out of the ticket writing in
order to be usable by the raw ticket reading function.
Refactor the existing DiscIO::AddTicket to not require the caller to
pass the requested title ID. We do not have the title ID in the ES case,
and it needs to be extracted from the ticket. Since this is always a
safe operation to do (title ID is always in the ticket), the
implementation is made default.
This constant isn't particularly helpful, mainly because it's not
applicable to all DSP instructions. Some instructions don't have encoding
space for registers, and not all instructions that do encode registers
have one at the first five bits.
This change also has the benefit of removing all includes to the
interpreter within the JIT code, which keeps them fully separate from one
another. Changes to the interpreter header won't require some of the JIT
code to be rebuilt.
Dolphin is able to generate one with all correct default settings, so
we don't need to ship with a pre-generated SYSCONF and worry about
syncing default settings.
Additionally, this commit changes SysConf to work with session SYSCONFs
so that Dolphin is able to generate a default one even for Movie/TAS.
Which SYSCONF needs to be touched is explicitly specified to avoid
confusion about which file SysConf is managing.
(Another notable change is that the Wii root functions are moved into
Core to prevent Common from depending on Core.)
Reset():
We only need to close IOS devices which were opened, and we can do that
simply by iterating over s_fdmap and closing any opened device.
With this change, s_device_map can be cleared at once.
SetDefaultContentFile():
We can just use s_es_handles which is guaranteed to contain three valid
ES devices. Gets rid of a downcast.
Since the Open command won't ever return with the stub, there's no way
we will get a Close/IOCtl/IOCtlV for it, so we don't have to
implement it at all.
is_hardware is an obscure name (what does hardware mean?) and it forces
us to assume that anything that !is_hardware is a FileIO device. This
assumption prevents properly restoring OH0 child devices (which will be
implemented in the USB PR), so this commit replaces the is_hardware
bool with a device type.
Confirmed by a hardware test and a quick diassembly of /dev/es.
I'm not aware of anything that opens several ES handles, but
technically, this fixes a small inaccuracy in IOS HLE.
For IOCTL_STM_EVENTHOOK, IOS checks if there is already an event hook
to prevent overriding an existing event hook message with a new one,
without first releasing it.
Currently, the country is always overridden depending on the Wii
language setting. This makes it impossible to change the country
independently from the language, unlike on a Wii, since a language
will always be associated to an unique country (which is hardcoded
in Dolphin's codebase).
This behaviour was added in c881262 and changed in PR 4319 to happen
every time emulation is started. It doesn't seem to be needed anymore,
as a quick testing shows that a Japanese system menu is able to load
in Japanese even with the region set to France, or anything other than
Japan in fact. So what this commit changes is drop this code.
Fixes issue 9884.
Removes #defines which have been unused for years and cleans up
naming.
This also changes IPC_REP_ASYNC to simply IPC_REPLY because it turns
out it's actually not specific to async replies, but used for all
command replies.
- Use an enum instead of defines.
- Only use the FS_ prefix for return codes which are actually related
to FS stuff, not for everything.
- Remove duplicated error codes and clean up the names.
Size is internally stored as a size_t, so having an int parameter
would cause implicit sign-conversion from a signed value to an
unsigned value to occur.
This removes Open() and Close() functions from devices whenever they
did nothing more than the base class (setting m_Active, returning a
default reply).
Also, since IOS close commands practically always return FS_SUCCESS,
writing the return code is moved to HandleCommand() in WII_IPC_HLE,
which has two benefits: it's not duplicated all over the place
(so people will not forget it) and it gets rid of having to check
the force parameter, since HandleCommand() is always called for
real IOS commands, so command_address is guaranteed to be valid.
It looks like at some point Dolphin device IDs coincided with IOS file
descriptors, but this is not the case anymore, so keeping those
WriteReturnCode(GetDeviceID()) in every single IOS HLE device,
as if the device ID was used as IOS fd, is both unnecessary
and confusing.
Jan 04 22:55:01 <leoetlino> fwiw, it looks like there are new warnings in the RegCache code
Jan 04 22:55:04 <leoetlino> Source/Core/Core/PowerPC/Jit64/FPURegCache.cpp:13:33: warning: declaration shadows a variable in the global namespace [-Wshadow]
Jan 04 22:56:19 <@Lioncash> yeah, the jit global should have a g_ prefix.
This fixes shadowing warnings and adds the g_ prefix to a global.
Other settings options are nouns rather than verbs so this change makes the configuration option consistent with others. Also makes the menu option label the same as the windows title.
The constructor shouldn't be used as a dumping ground for all UI-related
initialization. Doing so makes it somewhat more difficult to reason about
how certain UI elements get created. It also puts unrelated identifiers in
the same scope.
This separates the UI creation out so code relevant to each component
is self-contained.
We don't really have to keep track of device opens/closes manually,
since we can already check that by calling IsOpened() on the device.
This also replaces some loops with for range loops.
Certain parts of the standard library try to determine whether or not a
transfer operation should either be a copy or a move. The prevalent notion
of move constructors/assignment operators is that they should not throw,
they simply move an already existing resource somewhere else.
This is typically done with 'std::move_if_noexcept'. Like the name says,
if a type's move constructor is noexcept, then the functions retrieves an
r-value reference (for move semantics), or an l-value (for copy semantics)
if it is not noexcept.
As IOFile deletes the copy constructor and copy assignment operators,
using IOFile with certain parts of the standard library can fail in
unexcepted ways (especially when used with various container
implementations). This prevents that.
'operator void*' is basically a pre-C++11-ism that was used, as C++03
only had the notion of implicit type-conversion operators, but not explicit type
conversion operators (allowing implicit conversion of a file handle to
bool can go downhill pretty quickly).
ExecuteCommand was becoming pretty confusing with unused variables
for some commands, confusing names (device ID != IOS file descriptor),
duplicated checks, not keeping the indentation level low, and having
tons of things into a single function.
This commit gives more correct names to variables, deduplicates the
device checking code, and splits ExecuteCommand so that it's
easier to read.
It's worth noting that some device checks have been forgotten in the
past, which has caused a bug (which was recently fixed in 288e75f6).
GCMemcard is only used outside of the core and has
no real reason to use the core's RTC. GetEmulatedTime
isn't designed to be called when the core isn't running.
Should fix https://bugs.dolphin-emu.org/issues/9871
From what I can tell, the emulated GPU places (0,0) at the lower left of
the image, and we were generating texture coordinates so that (0,0) was
at the upper-left in the expansion geometry shader, causing textures
used by point sprites to be flipped vertically.
Fixes the upside-down A button in Mario Golf.
Since in this case we're setting it based on the state at record start
time, not when a register is loaded, UseMemory would not be called, so
this could potentially wipe out texture memory that was valid.
This should ensure that when playing with loop enabled, the first frame is
in the same state each time. There is potentially still issues when the
start frame is set to something other than zero, but I'm not sure how we
could work around this without capturing the entire state on each frame.
Doing it from the add dialogs instead would prevent the call to these dialogs outside of a breakpointWindow which would be necessary for hotkeys binding.
Instead of needing different switch cases for
converting countries to regions in multiple places,
we now only need a single country-to-region switch case
(in DiscIO/Enums.cpp), and we get a nice Region type.
This also makes it a strongly-typed enum.
Considering that the flushing mode is a trait/behavior for the register
cache, it doesn't really make sense to have the enum separate from it.
This also has the benefit of removing constants from global scope.
Lioncash