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

Merging Bug Fixes and Updates #61

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

tbare598
Copy link

@tbare598 tbare598 commented Mar 6, 2017

This merge isn't quite ready, but I wanted to create the pull request to start a discussion on some of the changes that I made, whether they were right to do, or not. Also, some of the code changes that I made directly conflict with changes that were made to the main branch, while I was working on my fork, including fixes to the same bugs, but the manner in which fixes were made, were much different. So we will have talk about which fixes to keep, and whether they are the right fixes going forward.


Edit:
The pull request is ready, and here are the notes for it:

Here are the bugs, I have fixed:

  • If a mutex had the number 3 or higher in the path, it wouldn't be removed.
  • The program was trying to kill mutexes on processes that no longer existed.
  • When using SmartSteamEmu, the steam ids were always the same, so some games would fail, when trying to play a game with a user with the same steam id.

Here are some enhancements I have made:

  • Left4Dead 2: Using the steam emulation, now allows the users to connect to each other's games, from within the game.
  • xinput folder allows users to share their controller profiles between games, much more easily.
  • Quicker game start up by removing a few unnecessary loops, and stopped the program from opening up ProcessUtils as a separate process.
  • Cleaned up some outdated comments
  • Added files to the .gitignore file, so that they won't be needlessly picked up in every commit. This also allows for cleaner merges.
  • When the link directories are created, the program will follow the paths and link all the files along each path.
  • New arguements to generic js files for better directory management.
  • The progam will now kill all game processes with the same name, before trying to open new game instances.
  • Added Earth Defense Force 4.1 to the game list

Below are my test results, after I merging all of the code, and as the code exists now. Some games, as noted, I do not own, and I was unable to test. Some other games did not work, but that is because they didn't even work before my code changes either.

  • Borderlands - Untested, I Don't Own This Game
  • Borderlands 2 - Working
  • Borderlands PreSequel - Working
  • Star Wars Battlefront 2 -

    Only player 1 opens.
    Screen resizes correctly, until the user navigates away from the main screen(when gameplay starts.
    Gamepad controls don't work correctly.

  • Earth Defense Force 4.1 - Works after manually changing settings.
  • Left 4 Dead 2 - Works but needs manual controller configuration until the xinput1_3.dll can be used to handle this.
  • Saints Row 3 - Can't get games to connect to each other.
  • Call of Duty: Black Ops - Untested, I Don't Own This Game
  • Call of Duty: Black Ops 2 - Untested, I Don't Own This Game

As for bringing the code changes into the main project branch, someone(probably lucasassislar), will have to rebase some of the commit revisions, so that the pull request, can be pulled in cleanly. Since lucasassislar made changes to the sames files as I did, there was going to be a number of conflicts. In an effort to make merging the pull request easier, I already took care of the conflicts, so we don't want git to try to merge the changes again.
To merge my pull request, this will have to be done: Revision 4b93a99 will have to be changed to point to revision e2435d2 as the next commit

tbare598 and others added 11 commits January 16, 2017 18:58
 - Removing some unused code and comments
Fixed some bugs with the generic handler that didn't account for games
where the XInput1_3.dll file, risided in a directory different from
the game's exe flie.
- For Left4Dead2, the save file was saving the word "True" and "False",
  instead of 1 or 0
- Mutexes were not being removed, if they had a three in the path.
- Giving each player a different x360ce.ini and xinput1_3.dll for each
  player will allow each instance to think that the cooresponding
  controller is "player 1" from within that instance.
While testing, I found a new max for session numbers, and that is
11. From a brief note found in the "Windows Sysinternals Admin Ref"
book, they only note that Sessions Id is >= 1. There has to be a
better way than looping through session id's, but for now, this
will be my simple fix.
Removed useless loops, refactored code, and removed dependancy on
running ProcessUtils as a separate program for checking for mutexes.
This bug might be fixed, and it seemed to happen because the program
was trying to access process handles that no longer existed.
- This required a change to the way steam ids where created with
  the use of the Steam Emulator.
Simple code formatting changes.
A user can now add their custom x360ce config files to a new xinput
directory, so that they can be added to each new game added.
@mirh
Copy link

mirh commented Mar 6, 2017

Github recommends to ignore both *.VC.db and *.VC.VC.opendb.

Also, "Cleaning Up Some Code"... Shouldn't be like mentioned that you are also changing toolset version too?

@tbare598
Copy link
Author

tbare598 commented Mar 6, 2017

  • Okay, I'll add *.VC.VC.opendb to the ignore file.
  • My Visual Studios client must have changed the tooling version without me noticing. If you think it will be a problem, I don't mind changing it back.

@mirh
Copy link

mirh commented Mar 6, 2017

Oh, no. VS 2015 is just fine for me.
It's just it seemed rude to "hide" such a -remotely potential- huge change.

EDIT: then I dunno what is Lucas using.

@tbare598
Copy link
Author

tbare598 commented Mar 6, 2017

Oh, no. I didn't mean to seem rude at all. I really really really want to help with this project, if at all possible. I love split screen gaming.

@mirh
Copy link

mirh commented Mar 12, 2017

LeftrDead2.js

I wanted to make this commit prior to merging in the changes by lucasassislar.
The reason being, I wanted to make sure that everything was working, and that
is is working similar to what is currently in the main branch. This is so that
when I make the merge, and conflicts are found(and there will be conflicts),
I can be very selective in what is merged in.

Changes that affect multiple files:
  1.0 Added better link directory/file creation logic. It will follow the relative
      paths provided, and create files on the way. After the links are created
      the hard files are copied over.

    1.1 This required more detail of the file locations, so I added the following
        variables to GenericGameInfo.cs, and in turn, the games' *.js files:
        pathExclusions, rootGameFolderPath, and xInputFolder

  2.0 Added the NeedsSteamEmulationDll variable to GenericGameInfo.cs, so that
      when a game has this set to true in their *.js file, a steam_api(64).dll
      file will be created in the directory where the SmartSteamLoader directory
      is placed. If there is a file with the name in that directory, it will be
      renamed to ValveApi(64).dll, before the steam_api(64).dll file is put into
      that directory. As a note, I am adding "(64)" to the file names in this
      comment, because the file will have the number 64 at the end of it, only if
      the game is 64 bit.

Battlefront2.js
	- I added NeedsSteamEmulationDll=true to this js config file, because I am
    able to open at least one game instance with and without steam being opened.
    Which is more than I have been able to without this code in place.
    > As a note, the first time I tried adding the steam_api.dll, I was able to
      get two instances to open up. So I implemented the ability to add the
      steam_api.dll based on the NeedsSteamEmulationDll variable. I may have been
      too quick to delete the directory and start over, because since that first
      time, I have been only been able to open one game instance at a time. Even
      with deleting different combinations of mutexes. I found this post on a forum
      and it looks like it is definatly possible to open two instances of BF2, it
      will just take some investigation:
        http://www.swbfgamers.com/index.php?topic=11251.0

EarthDefenseForce4.1.js
  - I added some useful TODOs to this *.js config file, about what has to happen
    for the game to work in splitscreen. Currently, this game works with the tool,
    but takes manual intervention after the game is started using this tool. One
    TODO that I would like to highlight, is that, maybe for this kinds of games,
    until we can fix the manual steps with the tool, we can add a pop up that
    tells the user what steps they have to take after they started the game.

Left4Dead2.js
  - I changed NeedsSteamEmulation=true, because I found that it works more reliably
    with the steam emulator.

SaintsRow3DX11.js
  - I changed the MaxPlayers=2, because this game only works for up to two players.
  - It looks like the players selection screen went down to three players, and not
    two players. So this will need to be fixed. I guess, with the current layout,
    we can just skip that screen when the game only supports two players, since
    that is also the minimum.
    > As a note, I would like to eventually add support for one player.
    > Another note, I would also love to have these screens pre-populated with the
      selections that were made the last time the game was started with this tool.

GenericGameHandler.cs
  - There are quite a bit of changes that where made to this file but most, of the
    changes have already been covered in sections 1-2 above.
  - Added a lot better Steam Emulator process handling. We now use the new function
    in ProcessUtils to find the game's process, which is a child process of the
    steam emulator, and then attach that to the player's process data. This allowed
    removal of the steam emulator code from the Update() procedure.

GenericGameInfo.cs
  - Added some default values.
  - Added PathExclusions, which will take the place of the exclusions variable, that
    was added to the main project branch.

CmdUtil.cs
  - Removed some logic from LinkDirectories(), that didn't seem to make sense, and
    was breaking when a directory was passed in, that used either "." or more
    importantly ".." to denote directories.
  - Added logic to LinkFiles(), to convert paths that had ".." and/or "." in them
    to the full path name.

ProcessUtil.cs
  - Added GetChildProcess(Process proc, List<string> childProcessPath) to this
    class. This function takes in a process, and a list of process names. The
    process names must be in the order of child processes, from the process that
    was also passed in, with the last process name in that list being the process
    that we want returned. If the function follows the list of children processes,
    and cannot find a child process that was in the list, it will return null.
@tbare598
Copy link
Author

Opps. I pushed the fix for that comment. It now correctly says Left4Dead2.js

lucasassislar and others added 5 commits March 12, 2017 23:25
Conflicts Resolved:
    Master/Games/Nucleus.Coop.Games.csproj
    Master/Games/games/Borderlands2.js
    Master/NucleusCoop.VC.db
    Master/NucleusGaming/Coop/Generic/GenericGameHandler.cs
- Adding -steam flag to start arguements, to stop the "Remove -insecure
  flag" error from appearing, on game startup.
- Setting "xinput1_3.dll" and "x360ce.ini" as XInputFiles, for better
  controller support, until we can modify the xinput1_3.dll to do what
  the x360ce.ini file if doing.
  Note: So to get the controllers to work properly for each game instance,
  there needs to be a different x360ce.ini file for player. Each
  x360ce.ini file needs to be configured so that its respective controller
  is considered "Player 1". Unfortunately, this process has to be manual,
  because there are GUIDs attached to each controller that is specific
  to the user's hardware.
…player: Added a tick that updates with 1000 Hz refresh rate to keep the user's mouse inside the game screen.

- CallOfDutyBlackOps.js, CallOfDutyBlackOps2Zombies.js: Initial files
- MainForm.cs: Double precision Thread.Sleep (to be able to sleep less than 1 ms)
- GenericGameHandler.cs: Everything related to time is now in double-precision. Added checks for null objects that could be caused by certain games not using all available features.
 * Fixed player id not being atached corrrectly
 * Reverted to extracting Steam Emu to the steam emu folder (not inside game).
 * Implemented Tick system: Generic Game asks us to tick the mentioned method a certain number of times during the game run.
 * CenterCursor(): Clips the mouse of the keyboard player inside his game window
 * Now HWnd stuff is updated only once a second
- GenericGameInfo.cs: Added HandlerInterval so games can modify it, removed old variable called Interval (was integer)
- IGenericGameInfo.cs: Now needs to provide a HandlerInterval double
- IGameHandler.cs: Updated all time to double
- PlayerInfo.cs: Added a variable holder for Player ID

(cherry picked from commit e90168a)
Removed all old code and games/games pics/useless DLLs and libs
* Program.cs: Removed all useless old code.
* Folder.cs: Renamed GameFolder to MainGameFolder, added new InstancedGameFolder variant (so we can get the main or the symlinked version of the folder).
* GenericContext.cs: Added SymlinkIgnore[] to be able to ignore custom files that are inside the main game installation (like video.txt on Left 4 Dead 2)
* GenericGameData: Renamed data to jsData (as it only holds the data that we want the JavaScript engine to be able to read). Reworked the order GenericContexts are created, so the JS has more data to work with. Added support for the SymlinkIgnore from the GenericContext, Source cfg files are now correctly handled and saved. Removed all mentions of HideTaskbar() as we don't use that method at all.
* Interop folder: refactoring everything here to organize the app/making new namespaces and just trying to follow some coding patterns to unfuck all this code (too much stuff copi pasted from stack overflow).
* SourceCfgFile.cs: Rewrote the file to work exactly like the IniFile class.
* GameManager.cs: Removed dynamic assembly reading (huge security flaw that doesn't need to be there). Games can only be added by custom JavaScript files now.
@tbare598
Copy link
Author

Okay, so the all of code from lucasassislar's project has been merged into my fork. Bugs and all. I cloned the current /lucasassislar/nucleuscoop project, and it looks like there was a bug introduced in the last commit with changing between the different types of same-screen split-screen. I'll fix that bug and add *.VC.VC.opendb to the .gitignore file as mentioned above, and then this pull request should be good to pull in.

  - Fixed the bug that caused the icon for the single-screen splitscreen
    to stay at the full screen mode, and not rotate through the different
    types, when clicked. This was done by keeping the code from resetting
    the screens to their defaults on every OnPaint call.
  - Generalized the gitignore to grab all *.VC.VC.opendb files, as well
    as adding *.VC.db to gitignore.
  - Removing this file from the repository, so that the .gitignore file
    can correctly skip this file.
.gitignore Outdated
@@ -211,4 +211,5 @@ FakesAssemblies/
GeneratedArtifacts/
_Pvt_Extensions/
ModelManifest.xml
/Master/NucleusCoop.VC.VC.opendb
**/*.VC.VC.opendb
Copy link

Choose a reason for hiding this comment

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

Why "paths"?
Every file with that extension, bar none, should be skipped.

Copy link
Author

@tbare598 tbare598 Mar 20, 2017

Choose a reason for hiding this comment

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

The pattern "**/*.VC.VC.opendb" is the same as "*.VC.VC.opendb". To be honest, I didn't realize that, when I made the commit, but I knew that "**/*.VC.VC.opendb" meant anywhere, including the current directory.

Edit: Apparently you have to escape asterisks in GitHub comments.

.gitignore Outdated
@@ -211,4 +211,5 @@ FakesAssemblies/
GeneratedArtifacts/
_Pvt_Extensions/
ModelManifest.xml
/Master/NucleusCoop.VC.VC.opendb
**/*.VC.VC.opendb
**/*.VC.db
Copy link

Choose a reason for hiding this comment

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

Didn't you added that extension already around 3rd comment?

Copy link
Author

Choose a reason for hiding this comment

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

I did, but while I was adding "*.VC.db" to the .gitignore file, I thought I should generalize both patterns, so that they would include any directory.

Copy link

Choose a reason for hiding this comment

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

"Extension" exclusion already handle that?

Copy link
Author

Choose a reason for hiding this comment

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

You are right. I'll clean that up.

  - Removing extraneous pattern: **/*.VC.db
  - Moving other pattern to a better location: *.VC.VC.opendb
@lucasassislar
Copy link
Owner

MY BAD.
I'm so so sorry, I wasn't keeping up with the Github page (in my mind I was trying to focus on giving feedback on the main subreddit and Discord channel). I just got back to reading all stuff here after I been notified, completely missed it :/

I'm taking a look at your changes and I can later discuss if you're still intersted.

@Jacalz
Copy link

Jacalz commented Apr 16, 2018

Really would like to see this merged if possible 👍

@wotever
Copy link

wotever commented Apr 11, 2019

for the games you dont own... dude, seriously. just pirate them. its only for testing purposes after all. no one needs to know. and you can delete this comment afterwards. they are old games anyway. or just buy them on a steam sale. they would be incredibly cheap already, and on sale would be super cheap. but at the end of the day, i am super happy with what you have done so far, it is absolutely amazing, thank you so much for this. you are a legend! :-)

clayne pushed a commit to clayne/nucleuscoop that referenced this pull request Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants