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

Add iOS version of Dolphin #3885

Closed
wants to merge 44 commits into from
Closed

Conversation

WilliamLCobb
Copy link

@WilliamLCobb WilliamLCobb commented Jun 6, 2016

I messed up the last pull request and it stopped updating commits.

Most of the changes are pretty straightforward.

Added iOS directory in Source.
Added new IOS definition when building for iOS
Added build steps for in readme.

CMake will generate an Xcode project in the build directory and copy in the Xcode project and workspace from the iOS directory. The workspace will combine these two projects, allowing full compatibility with future cmake and source changes while also giving control of all of the build settings for iOS. Since iOS is not inherently supported by cmake, this would have been very hard if not impossible without this step. The iOS app can be built in the Xcode GUI and also built from the command line.


This change is Reviewable

The contents of this directory will be copied into the build directory
post build.

These files make up the user interface and also handle host messages
iOS needs Prepare() for EAGL to know when to prepare it’s
frame buffer to be drawn into.

iOS also does not have a default frame buffer. GetDefaultFramebuffer()
will return the id of the frame buffer to draw into
Since iOS does not have <asm/hwcap.h> or <sys/auxv.h>, we let hardware
information remain uninitialized. This will need to be fixed
For std::array we needed <array>.
For sys_cache_control we needed <libkern/OSCacheControl.h>
On iOS pages will be mapped ANON and PRIVATE

Map the memory arenas as if we are on a 32 bit machine due to lack
of virtual memory
iOS controller handling is not finished yet but this is the skeleton
that will be used.
Since I was unable to fix MachineContext.h for iOS, I added empty
implementations for IOS until a fix can be added
m_flush_trigger.WaitFor() causes Dolphin to crash on iOS around every
other time a game is loaded. Remove for now and discover why this
causes a crash later
Added specific CMake configurations for IOS
iOS does not have <sys/disk.h>. These functions won’t be used so
setting DKIOCGETBLOCKCOUNT and DKIOCGETBLOCKSIZE to arbitrary values
serves as a workaround.
Updated readme to include system requirements and build steps for iOS
Those notices are automatically generated by Xcode
@lioncash
Copy link
Member

lioncash commented Jun 6, 2016

This includes quite a bit of external libraries as well, these should be in External, not pulled into the main tree.

Previously, WilliamLCobb (Will Cobb) wrote…

Add iOS version of Dolphin

I messed up the last pull request and it stopped updating commits.

Most of the changes are pretty straightforward.

Added iOS directory in Source.

Added new IOS definition when building for iOS

Added build steps for in readme.

CMake will generate an Xcode project in the build directory and copy in the Xcode project and workspace from the iOS directory. The workspace will combine these two projects, allowing full compatibility with future cmake and source changes while also giving control of all of the build settings for iOS. Since iOS is not inherently supported by cmake, this would have been very hard if not impossible without this step. The iOS app can be built in the Xcode GUI and also built from the command line.


Review status: 0 of 241 files reviewed at latest revision, 131 unresolved discussions, some commit checks failed.


Source/Core/Common/Arm64Emitter.cpp, line 15 [r1] (raw file):

#if APPLE_IOS
    #include <array>

This can just be placed with the regular standard library includes above.


Source/Core/Common/GL/GLInterface/EAGL.h, line 27 [r1] (raw file):

  void UpdateHandle(void* window_handle) override;
  void UpdateSurface() override;
    void Update() override;

These indentations should be tabs, not spaces.


Source/Core/Common/GL/GLInterface/EAGL.h, line 34 [r1] (raw file):

  void DetectMode();
  virtual void ShutdownPlatform() {}

unnecessary spaces here.


Source/Core/Common/GL/GLInterface/EAGL.mm, line 5 [r1] (raw file):

// Licensed under GPLv2+
// Refer to the license.txt file included.

This file should be using tabs for indentation, not spaces.


Source/Core/Common/GL/GLInterface/EAGL.mm, line 21 [r1] (raw file):

void cInterfaceEAGL::SwapInterval(int Interval)
{

Unnecessary spaces here.


Source/Core/Common/GL/GLInterface/EAGL.mm, line 40 [r1] (raw file):

    m_glkView = (GLKView *)window_handle;
    m_glkView.context = m_context;

Unnecessary spaces


Source/Core/Common/GL/GLInterface/EAGL.mm, line 52 [r1] (raw file):

void cInterfaceEAGL::Update()
{
    if (bufferStorageCreated) {

This should be removed if it's doing nothing useful.


Source/Core/Common/GL/GLInterface/EAGL.mm, line 61 [r1] (raw file):

{
    [m_glkView bindDrawable];
    if (!bufferStorageCreated) {

We use braces on the next line.


Source/Core/Core/MachineContext.h, line 39 [r1] (raw file):

      #error No context definition for OS
  #endif

Unnecessary newline.


Source/Core/Core/HW/GCMemcardRaw.cpp, line 86 [r1] (raw file):

      // If timed out, check if we need to flush.
#if APPLE_IOS
        bool do_exit = true; //WaitFor() crashes on iOS?

Indentation should be tabs here, and the comment should have a space after the //.


Source/Core/Core/PowerPC/JitArm64/JitArm64_BackPatch.cpp, line 336 [r1] (raw file):

  return true;
}

Unnecessary newline


Source/Core/DiscIO/DriveBlob.cpp, line 29 [r1] (raw file):

#include <sys/disk.h>   // DIOCGMEDIASIZE
#elif defined __APPLE__
    #if APPLE_IOS

Indentation should be tabs here.


Source/Core/InputCommon/CMakeLists.txt, line 17 [r1] (raw file):

elseif(APPLE_IOS)
  set(SRCS ${SRCS}
              ControllerInterface/iOS/iOS.mm

This should just be tabs, it's currently intermixing spaces.


Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp, line 73 [r1] (raw file):

#endif
#ifdef CIFACE_USE_APPLE_IOS
    ciface::iOS::Init(m_devices);

This should be indented using tabs.


Source/Core/InputCommon/ControllerInterface/iOS/iOS.h, line 1 [r1] (raw file):

// Copyright 2008 Dolphin Emulator Project

2016

This file should also be intended with tabs, not spaces.


Source/Core/InputCommon/ControllerInterface/iOS/iOS.h, line 22 [r1] (raw file):

        public:
            std::string GetName() const;
            Button(int padID, ButtonManager::ButtonType index) :  _padID(padID), _index(index) {}

one space too many after the : for _padID.


Source/Core/InputCommon/ControllerInterface/iOS/iOS.h, line 51 [r1] (raw file):

    };

}
} // namespace iOS
} // namespace ciface

Source/Core/InputCommon/ControllerInterface/iOS/iOS.mm, line 1 [r1] (raw file):

// Copyright 2013 Dolphin Emulator Project

2016


Source/Core/InputCommon/ControllerInterface/iOS/iOS.mm, line 16 [r1] (raw file):

{

void Init( std::vector<Core::Device*>& devices )

Opening and closing parameter list parentheses shouldn't be prefixed or suffixed with spaces.


Source/Core/InputCommon/ControllerInterface/iOS/iOS.mm, line 195 [r1] (raw file):

}

}
} // namespace iOS
} // namespace ciface

Source/Core/InputCommon/ControllerInterface/iOS/iOSButtonManager.cpp, line 14 [r1] (raw file):

namespace ButtonManager
{
  const std::string touchScreenKey = "Touchscreen";

We don't indent by one level in namespaces.


Source/Core/InputCommon/ControllerInterface/iOS/iOSButtonManager.h, line 12 [r1] (raw file):

namespace ButtonManager
{
  enum ButtonType

Ditto about not increasing initial indentation in namespaces.


Source/Core/InputCommon/ControllerInterface/iOS/iOSButtonManager.h, line 25 [r1] (raw file):

      BUTTON_LEFT = 8,
      BUTTON_RIGHT = 9,
      STICK_MAIN = 10, // Used on Java Side

This likely shouldn't mention Java, considering iOS isn't based on it.


Source/Core/InputCommon/ControllerInterface/iOS/iOSButtonManager.h, line 257 [r1] (raw file):

  void GamepadAxisEvent(const std::string& dev, int axis, float value);
  void Shutdown();
}

} // namespace ButtonManager


Source/Core/InputCommon/ControllerInterface/OSX/OSX.h, line 1 [r1] (raw file):

// Copyright 2016 Dolphin Emulator Project

This doesn't need to be changed. It only should be the current year for new files introduced into the repository.


Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm, line 1 [r1] (raw file):

// Copyright 2016 Dolphin Emulator Project

Ditto about this not needing to be changed


Source/Core/VideoBackends/OGL/PostProcessing.cpp, line 11 [r1] (raw file):

#include "Common/GL/GLUtil.h"
#include "Common/GL/GLInterfaceBase.h"

This include should go above GLUtil (I comes before U)


Source/Core/VideoBackends/OGL/PostProcessing.cpp, line 50 [r1] (raw file):

  ApplyShader();

Unnecessary newline, also the indentation should be tabs and not spaces for the preceding code.


Source/Core/VideoBackends/OGL/Render.cpp, line 1340 [r1] (raw file):

          // Tell the OSD Menu about the current internal resolution
          OSDInternalW = xfbSource->sourceRc.GetWidth(); OSDInternalH = xfbSource->sourceRc.GetHeight();
            GLInterface->Prepare();

Should be indented with tabs.


Source/Core/VideoBackends/OGL/Render.cpp, line 1350 [r1] (raw file):

      // for msaa mode, we must resolve the efb content to non-msaa
      GLuint tex = FramebufferManager::ResolveAndGetRenderTarget(rc);
        GLInterface->Prepare();

Should be indented with tabs.


Source/Core/VideoBackends/Software/SWOGLWindow.cpp, line 100 [r1] (raw file):

  GLInterface->Update();
  Prepare();
    GLInterface->Prepare();

Should be indented with tabs


Source/iOS/DolphiniOS/AppDelegate.m, line 7 [r1] (raw file):

//  Created by Will Cobb on 5/20/16.
//

File should be indented with tabs. Also braces on new line.


Source/iOS/DolphiniOS/AppDelegate.m, line 29 [r1] (raw file):

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
    // Override point for customization after application launch.

Unnecessary spaces on an empty line here.


Source/iOS/DolphiniOS/main.m, line 12 [r1] (raw file):

#import "AppDelegate.h"

int main(int argc, char * argv[]) {

Braces on new line, asterisk against the type, and indent with tabs.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 8 [r1] (raw file):

//  Copyright (c) 2015 OatmealDome. All rights reserved.
//

This file should indent with tabs and have braces on new lines.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 36 [r1] (raw file):

// Banner loading
//#include "DiscIO/BannerLoader.h"

This should be removed if it's not necessary.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 55 [r1] (raw file):

    File::CreateFullPath(File::GetUserPath(D_CONFIG_IDX));
    File::CreateFullPath(File::GetUserPath(D_GCUSER_IDX));
    //File::CreateFullPath(File::GetUserPath(D_WIIUSER_IDX));

Why is this commented out?


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 88 [r1] (raw file):

- (void) copyResources
{
    NSFileManager *fileManager = [NSFileManager defaultManager];

Asterisk goes against the type


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 92 [r1] (raw file):

    {
        NSLog(@"setting library directory to %@", self.getLibraryDirectory);
        NSError *err = nil;

Asterisk goes against the type


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 110 [r1] (raw file):

}

- (void) copyDirectoryOrFile:(NSFileManager*)fileMgr :(NSString *)directory

Asterisk goes against the type.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 112 [r1] (raw file):

- (void) copyDirectoryOrFile:(NSFileManager*)fileMgr :(NSString *)directory
{
    NSString *destination = [self.getLibraryDirectory stringByAppendingString:directory];

Asterisk goes against type


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 115 [r1] (raw file):

    NSString *source = [[[[NSBundle mainBundle] resourcePath] stringByAppendingString: @"/"] stringByAppendingString:directory];
    NSLog(@"copyDirectory source: %@", source);

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 116 [r1] (raw file):

    NSLog(@"copyDirectory source: %@", source);

    NSError *err = nil;

Asterisk goes against type


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 125 [r1] (raw file):

- (void) loadPreferences
{
    //

This is unnecessary


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 130 [r1] (raw file):

- (void) saveDefaultPreferences
{
    /*  Dolphin  */

// Dolphin


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 141 [r1] (raw file):

    dolphinConfig.GetOrCreateSection("Core")->Set("FrameSkip", "0x00000000");
    dolphinConfig.Save(File::GetUserPath(D_CONFIG_IDX) + "Dolphin.ini");

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 143 [r1] (raw file):

    NSLog(@"%s", (File::GetUserPath(D_CONFIG_IDX) + "Dolphin.ini").c_str());

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 145 [r1] (raw file):

    /*  OpenGL  */
    IniFile oglConfig;

// OpenGL


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 147 [r1] (raw file):

    IniFile oglConfig;
    oglConfig.Load(File::GetUserPath(D_CONFIG_IDX) + "gfx_opengl.ini");

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 148 [r1] (raw file):

    oglConfig.Load(File::GetUserPath(D_CONFIG_IDX) + "gfx_opengl.ini");

    IniFile::Section *oglSettings = oglConfig.GetOrCreateSection("Settings");

Asterisks go against the type.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 154 [r1] (raw file):

    oglSettings->Set("EnablePixelLighting", "False"); // originally false
    oglSettings->Set("DisableFog", "False");

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 155 [r1] (raw file):

    oglSettings->Set("DisableFog", "False");

    IniFile::Section *oglEnhancements = oglConfig.GetOrCreateSection("Enhancements");

Asterisks go against the type


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 163 [r1] (raw file):

    oglEnhancements->Set("StereoDepth", "20");
    oglEnhancements->Set("StereoConvergence", "20");

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 164 [r1] (raw file):

    oglEnhancements->Set("StereoConvergence", "20");

    IniFile::Section *oglHacks = oglConfig.GetOrCreateSection("Hacks");

Asterisk go against the type.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 171 [r1] (raw file):

    oglHacks->Set("EFBToTextureEnable", "True");
    oglHacks->Set("EFBCopyCacheEnable", "False");

Unnecessary spaces on an empty line


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 176 [r1] (raw file):

}

- (void)openRomAtPath:(NSString *)path;

Asterisks go against the type


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 181 [r1] (raw file):

    UICommon::SetUserDirectory([[self getUserDirectory] cStringUsingEncoding:NSUTF8StringEncoding]);
    UICommon::Init();

One unnecessary newline, and spaces on an empty line.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 221 [r1] (raw file):

void ConsoleListener::Log(LogTypes::LOG_LEVELS level, const char* text)
{
    switch(level)

switch (level)


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 242 [r1] (raw file):

}

void Host_NotifyMapLoaded() {}

The braces for these functions should be on new lines for consistency.


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 250 [r1] (raw file):

}

void Host_UpdateDisasmDialog(){}

same here and below.


Source/iOS/DolphiniOS/Bridge/DolphinGame.h, line 11 [r1] (raw file):

#import <UIKit/UIKit.h>

FOUNDATION_EXPORT NSString * const DolphinGameSaveStatesChangedNotification;

Asterisks go against the type.


Source/iOS/DolphiniOS/Bridge/DolphinGame.h, line 15 [r1] (raw file):

@interface DolphinGame : NSObject

@property (strong, nonatomic) NSString *path;

Asterisks go against the type, same for below.


Source/iOS/DolphiniOS/Bridge/DolphinGame.h, line 34 [r1] (raw file):

- (NSDate*)dateOfSaveStateAtIndex:(NSInteger)idx;
- (BOOL)deleteSaveStateAtIndex:(NSInteger)idx;
- (BOOL)deleteSaveStateWithName:(NSString *)name;

asterisk goes against the type.


Source/iOS/DolphiniOS/Bridge/DolphinGame.m, line 10 [r1] (raw file):

#import "Bridge/DolphinGame.h"

This file should be indented with tabs, have braces on the next line, have no spaces on empty lines, and have asterisks against the type.


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 1 [r1] (raw file):

// Copyright 2014 Dolphin Emulator Project

2016


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 24 [r1] (raw file):

namespace GCAdapter
{

One newline too many


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 30 [r1] (raw file):

}

One newline too many


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 44 [r1] (raw file):

void StopScanThread()
{

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 57 [r1] (raw file):

}

bool IsDetected() {

Brace on newline


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 60 [r1] (raw file):

    return 1;
}
bool IsDriverDetected() { return true; }

This should be on a newline and be spaced out like the others.


Source/iOS/DolphiniOS/Controller/GCAdapteriOS.cpp, line 79 [r1] (raw file):

void SetAdapterCallback(std::function<void(void)> func) { }

} // end of namespace GCAdapter

} // namespace GCAdapter


Source/iOS/DolphiniOS/Controller/GCControllerView.h, line 7 [r1] (raw file):

//  Created by Will Cobb on 5/31/16.
//

File should have asterisks against the type.


Source/iOS/DolphiniOS/Controller/GCControllerView.h, line 13 [r1] (raw file):

@protocol GCControllerViewDelegate <NSObject>

- (void)buttonStateChanged:(uint16_t)buttonState;

This can be u16 using Dolphin's typedefs in CommonTypes.


Source/iOS/DolphiniOS/Controller/GCControllerView.h, line 18 [r1] (raw file):

@interface GCControllerView : UIView {
    NSMutableArray *buttons;

indent with tabs


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 10 [r1] (raw file):

#import "Controller/GCControllerView.h"
#import "Controller/WCDirectionalControl.h"

File should indent with tabs and place asterisks against the type. Braces should be on a new line as well.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 33 [r1] (raw file):

    if (self = [super initWithFrame:frame]) {
        self.multipleTouchEnabled = YES;

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 35 [r1] (raw file):

        buttons = [NSMutableArray new];
        /*  Set up Buttons  */

// Set up buttons


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 36 [r1] (raw file):

        buttons = [NSMutableArray new];
        /*  Set up Buttons  */
        //A

// A


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 44 [r1] (raw file):

                           Image:@"gcpad_a.png"
                    PressedImage:@"gcpad_a_pressed.png"];
        //B

// B, etc for the others.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 52 [r1] (raw file):

                           Image:@"gcpad_b.png"
                    PressedImage:@"gcpad_b_pressed.png"];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 69 [r1] (raw file):

                           Image:@"gcpad_y.png"
                    PressedImage:@"gcpad_y_pressed.png"];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 86 [r1] (raw file):

                           Image:@"gcpad_r.png"
                    PressedImage:@"gcpad_r_pressed.png"];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 103 [r1] (raw file):

                           Image:@"gcpad_start.png"
                    PressedImage:@"gcpad_start_pressed.png"];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 114 [r1] (raw file):

                                [UIImage imageNamed:@"gcpad_dpad_pressed_left.png"],
                                [UIImage imageNamed:@"gcpad_dpad_pressed_upleft.png"]];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 121 [r1] (raw file):

        [self addSubview:dpad];
        [dpad addTarget:self action:@selector(dpadChanged:) forControlEvents:UIControlEventValueChanged];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 178 [r1] (raw file):

- (NSString *)getBitStringFor16:(uint16_t)value {
    NSString *bits = @"";

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/GCControllerView.m, line 182 [r1] (raw file):

        bits = [NSString stringWithFormat:@"%i%@", value & (1 << i) ? 1 : 0, bits];
    }

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCButtonControl.h, line 9 [r1] (raw file):

#import "Controller/WCDirectionalControl.h"

File should indent with tabs and have braces on a new line.


Source/iOS/DolphiniOS/Controller/WCButtonControl.m, line 9 [r1] (raw file):

#import "Controller/WCButtonControl.h"

File should indent with tabs, have braces on a new line, and put asterisks against the types.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.h, line 7 [r1] (raw file):

//  Created by Will Cobb on 5/31/16.
//

File should indent with tabs, have braces on a new line, and have asterisks against the type.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 7 [r1] (raw file):

//  Created by Will Cobb on 5/31/16.
//

File should be indented with tabs, have braces on a new line, and asterisks against types.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 13 [r1] (raw file):

    CALayer *buttonImage;
    CFTimeInterval lastMove;

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 34 [r1] (raw file):

        _backgroundImageView.image = [UIImage imageNamed:boundsImage];
        [self addSubview:_backgroundImageView];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 41 [r1] (raw file):

        buttonImage.actions = @{@"position": [NSNull null]};
        [self.layer addSublayer:buttonImage];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 43 [r1] (raw file):

        self.deadZone = CGSizeMake(MIN(self.frame.size.width/7, 20), MIN(self.frame.size.height/7, 20));

About two unnecessary newlines and unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 61 [r1] (raw file):

        _backgroundImageView.image = dpadImages[0];
        [self addSubview:_backgroundImageView];

One newline too many and unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 65 [r1] (raw file):

        self.deadZone = CGSizeMake(self.frame.size.width/3.1, self.frame.size.height/3.1);
        self.direction = 0;

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 113 [r1] (raw file):

    //if (!CGRectContainsPoint(self.bounds, loc)) return 0;
    WCDirectionalControlDirection direction = 0;

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 114 [r1] (raw file):

    WCDirectionalControlDirection direction = 0;

    if (loc.x > (self.bounds.size.width + self.deadZone.width)/2) direction |= WCDirectionalControlDirectionRight;
if (loc.x > (self.bounds.size.width + self.deadZone.width) / 2)
    direction |= WCDirectionalControlDirectionRight;
else if (loc.x < (self.bounds.size.width - self.deadZone.width) / 2)
    direction |= WCDirectionalControlDirectionLeft;

if (loc.y > (self.bounds.size.height + self.deadZone.height) / 2)
    direction |= WCDirectionalControlDirectionDown;
else if (loc.y < (self.bounds.size.height - self.deadZone.height) / 2)
    direction |= WCDirectionalControlDirectionUp;

Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 118 [r1] (raw file):

    if (loc.y > (self.bounds.size.height + self.deadZone.height)/2) direction |= WCDirectionalControlDirectionDown;
    else if (loc.y < (self.bounds.size.height - self.deadZone.height)/2) direction |= WCDirectionalControlDirectionUp;

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 122 [r1] (raw file):

        if (direction & WCDirectionalControlDirectionUp) {
            if (direction & WCDirectionalControlDirectionRight) {
                self.backgroundImageView.image = dpadImages[2]; //upleft

Comments should have a space after the //, same for others below.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 144 [r1] (raw file):

        }
    }

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 147 [r1] (raw file):

    return direction;
}

One newline too many


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 158 [r1] (raw file):

        self.direction = [self directionForTouch:touch];
    }

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 160 [r1] (raw file):

    [self sendActionsForControlEvents:UIControlEventValueChanged];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 181 [r1] (raw file):

        loc.x += self.bounds.size.width/2;
        loc.y += self.bounds.size.height/2;
        if (CACurrentMediaTime() - lastMove > 0.030) { // increasing this value reduces refresh rate and greatly increases performance

This comment can go above the if statment.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 188 [r1] (raw file):

        self.direction = [self directionForTouch:touch];
    }

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 200 [r1] (raw file):

        self.direction = [self directionForTouch:nil];
    }

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/UI/EmulatorViewController.h, line 10 [r1] (raw file):

#import <UIKit/UIKit.h>
@class GLKView;

Should have a newline after the include.


Source/iOS/DolphiniOS/UI/EmulatorViewController.h, line 14 [r1] (raw file):

@interface EmulatorViewController : UIViewController

@property IBOutlet GLKView *glkView;

Asterisk should go against the type, same for below.


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 25 [r1] (raw file):

#include "InputCommon/InputConfig.h"
#include "Core/HW/GCPadEmu.h"

Two unnecessary newlines


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 31 [r1] (raw file):

    DolphinBridge *bridge;
    GCControllerView *controllerView;

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 32 [r1] (raw file):

    GCControllerView *controllerView;

    GLuint texHandle[1];

Is this not able to be just a single non-array element.


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 48 [r1] (raw file):

- (void)viewDidLoad {
    [super viewDidLoad];

Unnecessary spaces on an empty line. Same for those below.


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 62 [r1] (raw file):

    bridge = [DolphinBridge new];

    //Add controller View

// Add controller view


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 72 [r1] (raw file):

{
    NSString *userDir = [bridge getUserDirectory];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 82 [r1] (raw file):

        [bridge saveDefaultPreferences];
    }
    //[bridge openRomAtPath:game.path];

This should be removed if it's unused (if it's planned to eventually be used, remove this and introduce it with the PR that makes use of it).


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 91 [r1] (raw file):

//Create a new class to handle the controller later

u16 buttonState;

These should be up with the


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 107 [r1] (raw file):

- (void)initController
{

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 121 [r1] (raw file):

    if (!portrait)
        return [UIScreen mainScreen].bounds.size;
    //Get portrait size

// Get portrait size


Source/iOS/DolphiniOS/UI/GameTableView.h, line 16 [r1] (raw file):

@interface GameTableView : UITableViewController

@property (atomic, strong) DolphinGame * game;

Asterisk should go against the type.


Source/iOS/DolphiniOS/UI/GameTableView.m, line 7 [r1] (raw file):

//  Created by Will Cobb on 11/4/15.
//  Copyright © 2015 iNDS. All rights reserved.
//

File should be indented with tabs, have asterisks against their types, and have braces on new lines.


Source/iOS/DolphiniOS/UI/GameTableView.m, line 8 [r1] (raw file):

//  Copyright © 2015 iNDS. All rights reserved.
//

One newline too many


Source/iOS/DolphiniOS/UI/GameTableView.m, line 18 [r1] (raw file):

@interface GameTableView() {

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/UI/GameTableView.m, line 34 [r1] (raw file):

    [super viewWillAppear:animated];
    [self.tableView reloadData];

Unnecessary spaces on an empty line.


Source/iOS/DolphiniOS/UI/GameTableView.m, line 59 [r1] (raw file):

-(NSArray *)tableView:(UITableView *)tableView editActionsForRowAtIndexPath:(NSIndexPath *)indexPath {

Unnecessary spaces on an empty line (same for others below)


Source/iOS/DolphiniOS/UI/RomTableViewController.h, line 13 [r1] (raw file):

@interface RomTableViewController : UITableViewController <UIAlertViewDelegate>
{
    NSArray *games;

Indent with tabs, braces against the type.


Source/iOS/DolphiniOS/UI/RomTableViewController.m, line 23 [r1] (raw file):

#import <mach/mach.h>
#import <mach/mach_host.h>

File should indent with tabs, have braces on new lines, and asterisks against the type.


Source/iOS/DolphiniOS/UI/RomTableViewController.m, line 38 [r1] (raw file):

    [[UIApplication sharedApplication] setStatusBarStyle:UIStatusBarStyleLightContent animated:NO];
    self.navigationItem.title = @"Roms";
    [SharkfoodMuteSwitchDetector shared]; //Start detecting

// Start detecting


Source/iOS/DolphiniOS/UI/RomTableViewController.m, line 39 [r1] (raw file):

    self.navigationItem.title = @"Roms";
    [SharkfoodMuteSwitchDetector shared]; //Start detecting

One newline too many


Source/iOS/DolphiniOS/UI/RomTableViewController.m, line 40 [r1] (raw file):

    [SharkfoodMuteSwitchDetector shared]; //Start detecting

Unnecessary spaces on an empty line (same for below)


Comments from Reviewable

@degasus
Copy link
Member

degasus commented Jun 6, 2016

Are all added files added on purpose? eg Source/iOS/DolphiniOS.xcodeproj/project.xcworkspace/xcuserdata/willcobb.xcuserdatad/ sounds wrong to me.

There are lots of whitespace vs tab issues on indenting. We are very nitpicking about this kind of issues :/

And it will very likely fail lint: https://github.com/dolphin-emu/dolphin/blob/master/Tools/lint.sh

Previously, lioncash (Mat M.) wrote…

This includes quite a bit of external libraries as well, these should be in External, not pulled into the main tree.


Reviewed 25 of 241 files at r1.
Review status: 25 of 241 files reviewed at latest revision, 140 unresolved discussions, some commit checks failed.


CMakeLists.txt, line 146 [r1] (raw file):

endif()

# If Dolphin is not built from a Git repository, default the version info to

likely a rebase error


Externals/SOIL/SOIL.c, line 25 [r1] (raw file):

      #include <GL/gl.h>
  #endif
#elif defined(__APPLE__) || defined(__APPLE_CC__)

&& !defined(APPLE_IOS)


Source/Core/AudioCommon/CoreAudioSoundStream.cpp, line 33 [r1] (raw file):

  desc.componentType = kAudioUnitType_Output;
#if APPLE_IOS
    desc.componentSubType = kAudioUnitSubType_GenericOutput;

whitespace vs tab


Source/Core/Common/MemArena.cpp, line 106 [r1] (raw file):

#if APPLE_IOS
    //ANON and PRIVATE are required for MAP_FIXED on iOS
    flags = MAP_ANON | MAP_PRIVATE | ((base == nullptr) ? 0 : MAP_FIXED);

MAP_ANONYMOUS, MAP_ANON is deprecated
Will MAP_ANONYMOUS break our mirrored memory mapping? For me, this sounds like this pages will be new mappings instead of the same as the other mirror.


Source/Core/Common/GL/GLInterfaceBase.h, line 41 [r1] (raw file):

  virtual bool ClearCurrent() { return true; }
  virtual void Shutdown() {}
  virtual void Prepare() {}

Can you give me a very small description on when to call Prepare()? How often? On which thread?


Source/Core/Common/GL/GLInterface/EAGL.mm, line 48 [r1] (raw file):

}

bool bufferStorageCreated = false;

also please a private member


Source/Core/DiscIO/DriveBlob.cpp, line 29 [r1] (raw file):

#include <sys/disk.h>   // DIOCGMEDIASIZE
#elif defined __APPLE__
    #if APPLE_IOS

#elif defined APPLE_IOS
......
#elif defined APPLE_
.......
#endif


Source/Core/InputCommon/ControllerInterface/ControllerInterface.h, line 29 [r1] (raw file):

  #endif
#endif
#if defined(__APPLE__)

#if defined(APPLE_IOS)
#define CIFACE_USE_EAGL
#elif defined(APPLE)


Source/Core/VideoBackends/OGL/Render.cpp, line 1340 [r1] (raw file):

          // Tell the OSD Menu about the current internal resolution
          OSDInternalW = xfbSource->sourceRc.GetWidth(); OSDInternalH = xfbSource->sourceRc.GetHeight();
            GLInterface->Prepare();

GLInterface->Prepare() can be moved into BlitScreen(), as it's called on all usages of PostProcessing::BlitScreen().


Comments from Reviewable

@BhaaLseN
Copy link
Member

BhaaLseN commented Jun 6, 2016

Review status: 25 of 241 files reviewed at latest revision, 147 unresolved discussions, some commit checks failed.


Source/Core/Common/GL/GLInterface/EAGL.mm, line 21 [r1] (raw file):

Previously, lioncash (Mat M.) wrote…

Unnecessary spaces here.

And other than the uppercase `Interval` parameter name (which should be a lower-case `interval`), is this method supposed to be empty? If so, please leave a note that it is intentional (or a FIXME comment with hints on what it should do for later if it isnt).

Source/Core/InputCommon/ControllerInterface/iOS/iOS.h, line 25 [r1] (raw file):

            ControlState GetState() const;
        private:
            const int _padID;

Not sure if the "single underscore at start is reserved" rule applies to OSX/iOS builds too, but I'd recommend following the general member-name guideline with the m_ prefix (such as m__padID, m_index etc)


Source/Core/InputCommon/ControllerInterface/iOS/iOS.mm, line 28 [r1] (raw file):

}

// Touchscreens and stuff

I don't think "Touchscreens and stuff" is a necessary nor useful comment here. At least in my eyes, it doesn't really add any value to it :/


Source/Core/InputCommon/ControllerInterface/iOS/iOSButtonManager.h, line 25 [r1] (raw file):

Previously, lioncash (Mat M.) wrote…

This likely shouldn't mention Java, considering iOS isn't based on it.

This file looks like a copy of the Android`ButtonManager` that resides in the JNI folder. Maybe those could be somehow merged and/or reused to avoid code duplication?

Source/iOS/DolphiniOS.xcodeproj/project.xcworkspace/xcshareddata/dolphin-emu.xcscmblueprint, line 18 [r1] (raw file):

  "DVTSourceControlWorkspaceBlueprintVersion" : 204,
  "DVTSourceControlWorkspaceBlueprintRelativePathToProjectKey" : "Source\/iOS\/dolphin-emu.xcodeproj",
  "DVTSourceControlWorkspaceBlueprintRemoteRepositoriesKey" : [

Not sure if those repository keys should be in here or not...but half of this file looks like it wouldn't necessarily work at any other machine


Source/iOS/DolphiniOS.xcodeproj/project.xcworkspace/xcuserdata/sc-wcobb-osx.xcuserdatad/UserInterfaceState.xcuserstate, line 0 [r1] (raw file):
Anything called user is probably not supposed to be under version control (especially when it includes your user/machine name)


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 183 [r1] (raw file):

    if (BootManager::BootCore([path UTF8String])) {

Braces on next line, or remove them (optional when both sides of if/else are single-line)


Source/iOS/DolphiniOS/UI/RomTableViewController.m, line 194 [r1] (raw file):

        gameInfo.game = game;
        [self.navigationController pushViewController:gameInfo animated:YES];
        [tableView deselectRowAtIndexPath:indexPath animated:YES];

Thats the same as inside the else, is that intended (which means you could move it outside and drop the else) or should it do something else?


Source/iOS/DolphiniOS/Utilities/Misc/UIViewController+BackButtonHandler.h, line 5 [r1] (raw file):

//
//  Created by Sergey Nikitenko on 10/1/13.
//  Copyright 2013 Sergey Nikitenko. All rights reserved.

Is this fine to include like that (unless it should be in Externals too)? Same for the SCLAlertView.


Comments from Reviewable

@WilliamLCobb
Copy link
Author

I fixed all whitespace and newline issues. Sorry about that.

I'll add .xcuserdatad and similar xcode files to gitignore.

All imported frameworks and libraries are not included in the source. They are found on the iPhone and are linked at runtime. Any other libraries or groups of files are purely UI enhancements and are iOS specific. I don't think they belong in external

Previously, degasus (Markus Wick) wrote…

Are all added files added on purpose? eg Source/iOS/DolphiniOS.xcodeproj/project.xcworkspace/xcuserdata/willcobb.xcuserdatad/ sounds wrong to me.

There are lots of whitespace vs tab issues on indenting. We are very nitpicking about this kind of issues :/

And it will very likely fail lint: https://github.com/dolphin-emu/dolphin/blob/master/Tools/lint.sh


Review status: 25 of 241 files reviewed at latest revision, 147 unresolved discussions, some commit checks failed.


CMakeLists.txt, line 146 [r1] (raw file):

Previously, degasus (Markus Wick) wrote…

likely a rebase error

Yea... not sure how that got removes. I added it back

Source/Core/Common/MemArena.cpp, line 106 [r1] (raw file):

MAP_ANONYMOUS
It doesn't seem to on iOS but I'll verify


Source/Core/Common/GL/GLInterfaceBase.h, line 41 [r1] (raw file):

Previously, degasus (Markus Wick) wrote…

Can you give me a very small description on when to call Prepare()? How often? On which thread?

I have to prepare the GLKView render buffer after the GLExtensions have been initiated (with GLExtensions::Init()) but before the first draw call every frame.

You would think I could allocate the buffers in MakeCurrent(), however, make current is called before GLExtensions::Init(), which causes the buffer to not work at all. I could hold off until later, but if you create it after the first draw call, it again fails to work properly.

I can create the frame buffer in GetDefaultFramebuffer() and instead, but videosw doesn't call it.


Source/Core/Common/GL/GLInterface/EAGL.mm, line 21 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

And other than the uppercase Interval parameter name (which should be a lower-case interval), is this method supposed to be empty? If so, please leave a note that it is intentional (or a FIXME comment with hints on what it should do for later if it isnt).

Removed

Source/Core/InputCommon/ControllerInterface/iOS/iOS.h, line 25 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

Not sure if the "single underscore at start is reserved" rule applies to OSX/iOS builds too, but I'd recommend following the general member-name guideline with the m_ prefix (such as m__padID, m_index etc)

This file was copied from the Android implementation. I think it should stay the same as Android until we decide if we should use a single implementation for both iOS and Android or not.

Source/Core/InputCommon/ControllerInterface/iOS/iOS.mm, line 28 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

I don't think "Touchscreens and stuff" is a necessary nor useful comment here. At least in my eyes, it doesn't really add any value to it :/

I'l remove it. This file was just copied over from the android implementation

Source/Core/InputCommon/ControllerInterface/iOS/iOSButtonManager.h, line 25 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

This file looks like a copy of the AndroidButtonManager that resides in the JNI folder. Maybe those could be somehow merged and/or reused to avoid code duplication?

I plan to edit it. Right not it is just place holding. If the code ends up being very similar, I'll ask to merge these into a common folder to reduce code duplication.

Source/Core/VideoBackends/OGL/Render.cpp, line 1340 [r1] (raw file):

GLInterface->Prepare()
Moved to OpenGLPostProcessing::BlitFromTexture(). I can probably combine Prepare() with GetDefaultFramebuffer() if you prefer


Source/iOS/DolphiniOS.xcodeproj/project.xcworkspace/xcshareddata/dolphin-emu.xcscmblueprint, line 18 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

Not sure if those repository keys should be in here or not...but half of this file looks like it wouldn't necessarily work at any other machine

These are generated by Xcode and remade when run on a different machine. I'll add them the gitignore

Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 55 [r1] (raw file):

Previously, lioncash (Mat M.) wrote…

Why is this commented out?

Removed

Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 82 [r1] (raw file):

Previously, lioncash (Mat M.) wrote…

This should be removed if it's unused (if it's planned to eventually be used, remove this and introduce it with the PR that makes use of it).

Removed

Source/iOS/DolphiniOS/Utilities/Misc/UIViewController+BackButtonHandler.h, line 5 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

Is this fine to include like that (unless it should be in Externals too)? Same for the SCLAlertView.

Xcode automatically generates these comments. I'll check the license for the library and remove it if it's not compatible with GPL

Comments from Reviewable

@degasus
Copy link
Member

degasus commented Jun 6, 2016

Reviewed 2 of 5 files at r7.
Review status: 26 of 165 files reviewed at latest revision, 135 unresolved discussions, some commit checks failed.


Comments from Reviewable

@OatmealDome
Copy link
Member

@degasus Will do.

Importing common types into an objc file was causing compiler errors.
I’ll use uint16_t instead of u16 until this is fixed
@OatmealDome
Copy link
Member

Some things:

  • I was not able to build at first due to some odd (but familiar) errors with zlib; I had to apply this patch to get it working.
  • The default development team in Xcode is set to an unknown team. (I assume yours?) Perhaps we should change it so that no team is set by default.
  • ZAActivityBar and RBVolumeButtons use a bunch of methods/properties that have been deprecated since iOS 7.
  • Some odd warnings: (the final one is repeated for each .o file in each .a file.)
ld: warning: directory not found for option '-L/Volumes/Shared Data/Development/dolphin-cobb/Build/Libraries'
ld: warning: directory not found for option '-L/Volumes/Shared Data/Development/dolphin-cobb/Build/Externals/gtest/Debug-iphoneos'
ld: warning: directory not found for option '-L/Volumes/Shared Data/Development/dolphin-cobb/Build/Source/Core/UICommon/Debug-iphoneos'
ld: warning: directory not found for option '-F/Volumes/Shared Data/Development/dolphin-cobb/Build/DolphiniOS/Frameworks'
warning: All interface orientations must be supported unless the app requires full screen.
ld: warning: object file (/Users/mac/Library/Developer/Xcode/DerivedData/DolphiniOS-dxdqzgwufyakuugcegupxmjghyfb/Build/Products/Debug-iphoneos/libcommon.a(StringUtil.o)) was built for newer iOS version (9.3) than being linked (8.1)
  • A directory named 'Roms' is expected to be in Build/DolphiniOS, and the build fails if Xcode can't find it. I had to make it myself.
  • There is no reuse identifier for the prototype cell in RomTableViewController.
  • drawInRect isn't implemented for EmulaterViewController (which is a GLKViewDelegate).
  • The UI is very barebones, but I guess that's to be expected at this stage. Here it is running on an iPad Mini. The only real nitpick I have is that imo Roms should be ROMs.
  • In the application code, there are a bunch of strings that we should be able to localize. I'm not very familiar with how Dolphin's localization process works, so there could be something we need to change. In a normal iOS app, we would use NSLocalizedString(str, comment) along with a Localizable.strings file.

Previously, OatmealDome wrote…

@degasus Will do.


Review status: 25 of 165 files reviewed at latest revision, 147 unresolved discussions, some commit checks failed.


Source/iOS/DolphiniOS/AppDelegate.h, line 19 [r3] (raw file):

@property (strong, nonatomic) UIWindow *window;
@property (strong, nonatomic, getter=getSettingsViewController) WCEasySettingsViewController *settingsViewController;
@property EmulatorViewController  * currentEmulationController;

why is there an indent mid-way through the line?


Source/iOS/DolphiniOS/AppDelegate.h, line 24 [r5] (raw file):

- (NSString*)cheatsDir;
- (NSString*)batteryDir;

"batteryDir" might not be the best name, considering that we will store save states here. additionally, these don't need to be - and can be +, removing part of the need for the sharedInstance var


Source/iOS/DolphiniOS/AppDelegate.m, line 74 [r4] (raw file):

{
  dispatch_async(dispatch_get_main_queue(), ^{
      SCLAlertView*  alertView = [[SCLAlertView alloc] init];

extra space


Source/iOS/DolphiniOS/Bridge/DolphinBridge.h, line 22 [r5] (raw file):

- (void) saveDefaultPreferences;
- (void) openRomAtPath:(NSString *)path;
- (void) redirectConsole;

no method declarations for redirectConsole and loadPreferences


Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 7 [r4] (raw file):

//  Created by mac on 2015-03-17.
//  Copyright (c) 2015 OatmealDome. All rights reserved.
//

should we shorten these xcode generated headers to the standard

// Copyright 2016 OatmealDome, WillCobb
// Licensed under GPLV2+
// Refer to the license.txt provided

?

The comment below giving me credit can then be removed if this happens.


Source/iOS/DolphiniOS/Bridge/DolphinGame.h, line 24 [r5] (raw file):

@property (nonatomic, readonly) BOOL      hasPauseState;

+ (int)preferredLanguage; // returns a NDS_FW_LANG_ constant

NDS?


Source/iOS/DolphiniOS/Bridge/DolphinGame.m, line 10 [r5] (raw file):

#import "Bridge/DolphinGame.h"

This file appears to have some leftovers from iNDS


Source/iOS/DolphiniOS/Bridge/DolphinGame.m, line 39 [r5] (raw file):

      else
      {
          //Recurse

space after //


Source/iOS/DolphiniOS/Controller/WCDirectionalControl.m, line 50 [r4] (raw file):

}

//Accepts 1, 5, or 9 images

space after //


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 50 [r5] (raw file):

  v = self.glkView;
  v.delegate = self;

can't we just use self.glkView.delegate = self and then get rid of v?


Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 69 [r5] (raw file):

      // let's setup everything
      NSString* docDir = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES)[0];
      [bridge setUserDirectory:[docDir stringByAppendingString:@"/Dolphin"]];

can't you just call documentsDir in AppDelegate?


Source/iOS/DolphiniOS/UI/GameTableView.m, line 58 [r5] (raw file):

          NSString* savePath = [_game pathForSaveStateAtIndex:indexPath.row];
          NSString* rootPath = savePath.stringByDeletingPathExtension.stringByDeletingPathExtension;
          NSString* dstPath = [NSString stringWithFormat:@"%@.%@.dsv", rootPath, textField.text];

iNDS leftover?


Comments from Reviewable

@degasus
Copy link
Member

degasus commented Jun 7, 2016

Reviewed 1 of 108 files at r2, 1 of 2 files at r8.
Review status: 26 of 165 files reviewed at latest revision, 147 unresolved discussions, some commit checks failed.


Source/Core/Core/HW/GCMemcardRaw.cpp, line 85 [r8] (raw file):

      // If triggered, we're exiting.
      // If timed out, check if we need to flush.
#if APPLE_IOS

This should not be needed at all. Maybe this here shows an issue somewhere else...


Comments from Reviewable

@WilliamLCobb
Copy link
Author

Thats odd, I'll apply that patch.

I removed ZAActivityBar and a few other libraries that we don't really need. RBVolumeButtons lets you use the volume buttons as the left and right trigger for games. It uses some old libraries that let you intercept volume presses. I'll see if updating depreciated APIs is possible

I can't get the Xcode variable SRCROOT to point to the actual source root. It points to the parent directory, and I think something related is causing those warnings. Since the build and source directory are "($SRCROOT)/Build" instead of just "($SRCROOT)". If you build in a directory not named "Build", the xcode project will have the wrong path for files. If you know how to fix that @OatmealDome, that would be awesome.

I'll be making all of the suggested UI changes. I also removed a bunch of stuff left over from iNDS (and the project it was adapted from) like you suggested. I really should have cleaned up the code before the pull request. Also I'll get rid of the weird AppDelegate shared instance usage.

Previously, OatmealDome wrote…

Some things:

  • I was not able to build at first due to some odd (but familiar) errors with zlib; I had to apply this patch to get it working.
  • The default development team in Xcode is set to an unknown team. (I assume yours?) Perhaps we should change it so that no team is set by default.
  • ZAActivityBar and RBVolumeButtons use a bunch of methods/properties that have been deprecated since iOS 7.
  • Some odd warnings: (the final one is repeated for each .o file in each .a file.)
ld: warning: directory not found for option '-L/Volumes/Shared Data/Development/dolphin-cobb/Build/Libraries'
ld: warning: directory not found for option '-L/Volumes/Shared Data/Development/dolphin-cobb/Build/Externals/gtest/Debug-iphoneos'
ld: warning: directory not found for option '-L/Volumes/Shared Data/Development/dolphin-cobb/Build/Source/Core/UICommon/Debug-iphoneos'
ld: warning: directory not found for option '-F/Volumes/Shared Data/Development/dolphin-cobb/Build/DolphiniOS/Frameworks'
warning: All interface orientations must be supported unless the app requires full screen.
ld: warning: object file (/Users/mac/Library/Developer/Xcode/DerivedData/DolphiniOS-dxdqzgwufyakuugcegupxmjghyfb/Build/Products/Debug-iphoneos/libcommon.a(StringUtil.o)) was built for newer iOS version (9.3) than being linked (8.1)
  • A directory named 'Roms' is expected to be in Build/DolphiniOS, and the build fails if Xcode can't find it. I had to make it myself.
  • There is no reuse identifier for the prototype cell in RomTableViewController.
  • drawInRect isn't implemented for EmulaterViewController (which is a GLKViewDelegate).
  • The UI is very barebones, but I guess that's to be expected at this stage. Here it is running on an iPad Mini. The only real nitpick I have is that imo Roms should be ROMs.
  • In the application code, there are a bunch of strings that we should be able to localize. I'm not very familiar with how Dolphin's localization process works, so there could be something we need to change. In a normal iOS app, we would use NSLocalizedString(str, comment) along with a Localizable.strings file.

Review status: 26 of 165 files reviewed at latest revision, 147 unresolved discussions, some commit checks failed.


Source/Core/Core/HW/GCMemcardRaw.cpp, line 85 [r8] (raw file):

Previously, degasus (Markus Wick) wrote…

This should not be needed at all. Maybe this here shows an issue somewhere else...

I can't figure it out. It works 2/3 of the time but gives a bad access error at 0x0 around every third game thats booted up. Removing this line completely fixes it. I'll see today if I can find the problem

Source/iOS/DolphiniOS/AppDelegate.h, line 24 [r5] (raw file):

Previously, OatmealDome wrote…

"batteryDir" might not be the best name, considering that we will store save states here. additionally, these don't need to be - and can be +, removing part of the need for the sharedInstance var

Done.

Source/iOS/DolphiniOS/Bridge/DolphinBridge.mm, line 7 [r4] (raw file):

Previously, OatmealDome wrote…

should we shorten these xcode generated headers to the standard

// Copyright 2016 OatmealDome, WillCobb
// Licensed under GPLV2+
// Refer to the license.txt provided

?

The comment below giving me credit can then be removed if this happens.

I'm fine with this. I'll add this header to all of the files

Source/iOS/DolphiniOS/Bridge/DolphinGame.h, line 24 [r5] (raw file):

Previously, OatmealDome wrote…

NDS?

Nds references and related code have been removed

Source/iOS/DolphiniOS/UI/EmulatorViewController.mm, line 50 [r5] (raw file):

Previously, OatmealDome wrote…

can't we just use self.glkView.delegate = self and then get rid of v?

Yep. I ended up passing the view to DolphinBridge. It follows your design of limiting most or all of the calls to Dolphin methods to DolphinBridge and interfacing them with Objective-c methods.

Comments from Reviewable

@OatmealDome
Copy link
Member

@WilliamLCobb I took a look, but honestly I have no idea what's wrong with $SRCROOT. Not even my google-fu could help. There might be something wrong with the xcodeproj file, but I'm not 100% sure. Perhaps you could create a new project, copy/paste the existing source files in, and see if the new project works?

Previously, WilliamLCobb (Will Cobb) wrote…

Thats odd, I'll apply that patch.

I removed ZAActivityBar and a few other libraries that we don't really need. RBVolumeButtons lets you use the volume buttons as the left and right trigger for games. It uses some old libraries that let you intercept volume presses. I'll see if updating depreciated APIs is possible

I can't get the Xcode variable SRCROOT to point to the actual source root. It points to the parent directory, and I think something related is causing those warnings. Since the build and source directory are "($SRCROOT)/Build" instead of just "($SRCROOT)". If you build in a directory not named "Build", the xcode project will have the wrong path for files. If you know how to fix that @OatmealDome, that would be awesome.

I'll be making all of the suggested UI changes. I also removed a bunch of stuff left over from iNDS (and the project it was adapted from) like you suggested. I really should have cleaned up the code before the pull request. Also I'll get rid of the weird AppDelegate shared instance usage.


Review status: 26 of 120 files reviewed at latest revision, 146 unresolved discussions, some commit checks failed.


Comments from Reviewable

For some reason, a string as the second argument in Set() would match
the overloaded function using that string as a bool. This always
evaluated to True and wrote “True” to the setting. In order for the
string to be correctly handled, you must wrap it in std::string()
openRomAtPath:inView was blocking the main thread. It now correctly
dispatches a block to a background queue.
@WilliamLCobb
Copy link
Author

What caused the OSX build to fail?

iOS and Android were using the exact same images in 2 different
folders. This lets iOS support using a shared folder with android.
Android support for this shared folder will need to be added in a later
pr
@Fallcrest
Copy link

Just a question, does this build of Dolphin support the Apple TV? The underlying tvOS operating system is, as far as I know, functionally identical, but the UI would need to be tweaked for Dolphin to run on Apple TV, as it does not have a touchscreen for inputs.

Would having an iOS version be reason to seriously consider writing a Metal video backend (Issue 9510)? It could provide possibly necessary performance increases.

It is very impressive what has already been done for such a limited device. Reading up on OatmealDome's original forum thread made it seem like the memory was far too limited to get anything playable. Maybe some of this code could be adapted to help with lower memory Android and PC devices as well?

@OatmealDome
Copy link
Member

OatmealDome commented Jun 16, 2016

@Fallcrest There is no support for the Apple TV at this time.

I'd say that if this is merged, it can be a big motivating factor for someone to take up the task of writing a Metal backend. It could save us from the horrible OpenGL support from Apple. (Apparently, there are no changes to OpenGL in macOS Sierra, which sucks.)

The devices should have enough RAM; the problem I encountered is that operating system limits us on how we can use/"organize" that memory. About adopting the new code: There isn't really any big change in this PR for how Dolphin handles memory. We simply just initialize the memory map like we're running on a 32-bit device.

If there's anything wrong in this post, feel free to correct me.

@Fallcrest
Copy link

Fallcrest commented Jun 16, 2016

@OatmealDome Oh yeah... "macOS" Apple again changed the branding. (looking back at all of their old operating systems shows how many times they have changed the naming scheme. It just doesn't make sense.) Someone will have to change that in Dolphin's code/documentation now, won't they...

My apologies for misunderstanding the memory management part. I am most certainly not knowledgeable on the topic, I have just been reading a lot of other people's work and I thought that this was connected to other things.

Apple has pretty much dropped support for new versions of OpenGL, as far as I know. But then again, they never kept it up to date and never really talked about it, so who knows if they have any plans. Because of their focus on Metal, I doubt they will give OpenGL a second thought, though. I wish they had transitioned to Vulkan which would have been far more useful to Dolphin (and to practically all developers of anything) because of its cross-platform nature. But for now at least, we are stuck with Metal being the only actively supported solution on Apple systems. Which ends up hurting the adoption of Vulkan significantly. Sigh...

Even though Vulkan would only be available for the Windows, Linux, and Android builds, (and possibly FreeBSD, I don't really know how drivers are released for there), it still might be something worth looking into as the maintainers of OpenGL and Vulkan will eventually transition away from OpenGL in favor of Vulkan. It would probably be many years before they do that, but it is worth future proofing ourselves, especially when Vulkan has proven to have very good performance. There is also some project called MoltenVK that is working on getting Vulkan to run on top of Metal, so the same API could be used for Apple products with minimal extra work. It is sadly a closed commercial product, so it could not be used here, but the idea might still be useful.

It is very late and I have rambled way too much, most of this is probably irrelevant. I am very excited for this PR, iOS was the last major market, so to speak, that Dolphin needed to get into. (Looking forward to Dolphin being ported to various VR systems next. I won't even benefit from that, it will just be very cool.)

@WilliamLCobb
Copy link
Author

This should be ready to merge once the git issues for the OSX build are fixed.

If it looks like a Metal backend will provide a significant speedup, I'll write a basic implementation after this has been merged.

@Fallcrest
Copy link

@WilliamLCobb Okay, I was beginning to look into a Metal backend, but you will almost certainly finish this faster than I would, plus you have the ability to test this on iOS. I was doing some preliminary research while waiting for several PRs to simplify VideoCommons to make the job easier. I might look into doing Vulkan instead. I'm happy that 5.0 is finally out. There are some really cool ideas that will be implemented in the (hopefully) near future.

@OatmealDome
Copy link
Member

OatmealDome commented Jun 24, 2016

Funny you started looking into a backend, @Fallcrest.. I actually started looking into the feasibility of a Metal backend too a few days after I posted that comment lol. It seems perfectly feasible to me. (After my research on how Dolphin's VideoBackends work with VideoCommon... I support your future PRs to clean it up. They would be helpful to me and to future devs.)

By lurking on the IRC, I found that the full message from that other dev was that the "C++ bindings were too slow". The simplest solution here is to just call Obj-C stuff from Obj-C++ files. (Perhaps we can add Obj-C++ back to the WIkipedia page if this happens, neobrain? :P)

@Fallcrest
Copy link

@OatmealDome Well, I was beaten to Vulkan as well, but that's fine. Perhaps you or @WilliamLCobb could open up a minimal PR for Metal just so others know that it is being worked on, and so they can voice their opinions.

@grp grp mentioned this pull request Jun 26, 2016
@impiaaa
Copy link

impiaaa commented Jun 26, 2016

degauss told me to post this: here's the very beginnings of a Metal backend (Mac only).

@WilliamLCobb
Copy link
Author

WilliamLCobb commented Jun 29, 2016

New iOS pull request: #3941

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

Successfully merging this pull request may close these issues.

7 participants