Pull in Greg's changes #7
Conversation
…ing it more in-line with modern standards.
…ern compiler standards; Remove some redundant files.
…pes had to be suppressed because its a specialized situation.
…idea. Reverting back to smaller window.
…iler and linker).
…ffer (again); minor formatting.
…ars ago. At that time, I was testing on huge and powerful speakers with a very different (real) decoupling capacitor size. Turns out, this change had a terrible effect on synthesized music audio.
…has a dramatic effect on the smoothness of the Mockingboard sound (sounds more like a real Mockingboard).
…2. Perhaps if we upgrade to a newer SDL in the future, this can be revisited.
Merge Andrey's 2b version
This took a bit of doing and will require some testing to make sure I didn't accidentally revert anything I shouldn't have, but I think I did okay. Testing wanted before I merge it into linappleii/linapple.
It helps when you tell apt-get what you want to do!
This is NOT the best way to do this. Whatever, make it work for the moment. We'll do it right later.
There are probably more droppings that Greg didn't remove from his tree but could have. I just happen to know these aren't used anymore. :) Changed the README to push people toward the use of fakeroot to build the Debian package. Using sudo makes it so that git cannot modify your working directory. To fix this before you pull, run: sudo chown -r <you> pkg Obviously replacing <you> with your username. Actually, if your working directory is clean (git status), you can: sudo rm -rf pkg git checkout -f That'll rebuild the pkg directory properly. To avoid the problem in the future, use fakeroot rather than sudo to make package and the system will just pretend to chown everything as root for the purposes of running dpkg-deb on it. In future, we'll set up a proper debian/ dir and the make pkg line will use dpkg-buildpackage or gbp as necessary to build the package. This should bring us up to something that can be merged.
|
I'll leave this open at least a few days so that y'all have a chance to look at it before I merge. In the meantime, I'm branching off of this tree to begin the quick and dirty SDL2 port using SDLCL. |
In Applewin.cpp, that's genunely an un-initialized variable. Arlo probably commented out the ptm assignment not knowing immediately what to do with it and planning to come back later. For ftpparse.cpp, t should never be returned outside the for loop which assigns it, but it hurts nothing to initialize t to silence the warning.
|
I haven't given it a thorough look yet, but the following may be helpful: When viewing the changes, the "Diff settings" button > "Hide whitespace changes" does help. inc/.SerialComms.h.swp and inc/.MouseInterface.h.swp might be temp files that were committed by accident? A bit surprised that the directory for header files is called Absolutely no idea yet how this code manages runtime resource dependencies (splash screen BMP, etc) versus how my fork does. But also, my use case was that linapple should be runnable from any directory simply by being on the search path, without requiring to be installed system-wide. I tested that use case on this repository yesterday (I'm using this as an excuse to fix the Apple II support in Funicular) and it worked alright. I'll test that use case against this branch when I get the opportunity too. Thanks! |
|
inc/.MouseInterface.h.swp: Vim swap file, version 7.4, pid 15907, user ghedger, host ghedger-W65-67SZ, file /u1404/home/ghedger/projects/apple2/git/linapple/inc/MouseInterface.h Right you are, removed. SDL2 provides some interesting functions for our purposes, SDL_GetBasePath and SDL_GetPrefPath. The former is the directory the application lives in. This doesn't 100% map to the "UNIXy way" of doing things, but it allows you to identify where to find things like the charset. SDL_GetPrefPath returns a user directory suitable for storing application data of various sorts—~/.local rather than ~/.config. Much wailing there was about not properly following XDG to the letter, but it was noted that SDL is often used to port games from windows where that kind of fine-grained distinction doesn't always exist. I assume we use both. We check for the latter, then the former. Anything we write goes into the user directory. SDL_GetBasePath returns the directory the application resides in (/usr/local/linapple or wherever), |
inc/Common.h
Outdated
| #define TITLE_APPLE_2_PLUS TEXT("Apple ][+ Emulator") | ||
| #define TITLE_APPLE_2E TEXT("Apple //e Emulator") | ||
| #define TITLE_APPLE_2E_ENHANCED TEXT("Enhanced Apple //e Emulator") | ||
| #define TITLE_APPLE_2 ("Apple ][ Emulator") |
There was a problem hiding this comment.
Based on the fact that it's still there on the following lines, it looks like this TEXT was removed accidentally?
There was a problem hiding this comment.
Yeah I noticed that. And then forgot it again. Thanks!
Makefile
Outdated
| #Link | ||
|
|
||
| $(TARGET): $(OBJECTS) | ||
| $(CC) $(LFLAGS) -o $(TARGETDIR)/$(TARGET) $^ $(LIB) |
There was a problem hiding this comment.
If you run make (no arguments) after everything is already built, linking happens again, i.e. this rule gets applied again.
This isn't a huge deal, but what I'd expect to see instead is something like "make: nothing to be done."
I think this is because this rule relies on $(TARGET) (which is linapple) but produces a file called $(TARGETDIR)/$(TARGET).
Two ways to fix it: make this rule rely on $(TARGETDIR)/$(TARGET), or include the path in $(TARGET).
I lean towards the latter, but I am not really a Makefile person, and as I said it's pretty minor.
There was a problem hiding this comment.
Are you opposed to autoconf or even automake? Some people are fanatically opposed because of how ridiculously overcomplicated it is, particularly in terms of what it generates, in the name of compatibility with 60 year old UNIX systems nobody's using anymore. I tried to love CMake, and I certainly love how it can generate all the project files for all the platforms, but it always felt like I was wrestling with it. Maybe I'm just more familiar with autoconf and potentially automake.
Which isn't to say that you don't wrestle with those too—I'm just probably quite able to write a configure.ac in an afternoon despite the fact that I've not done it for ten years.
LinApple's build isn't so complex that it really requires it, but if we want things to behave properly UNIXy, it doesn't get any more (pathologically) UNIX than autoconf at least. ;)
linapple.conf
Outdated
| @@ -1,253 +0,0 @@ | |||
| # | |||
| # linapple.conf - config file used by LinApple, Apple][ (Apple2, Apple 2) emulator for Linux and other POSIX systems with SDL support | |||
There was a problem hiding this comment.
I was wondering why this diff didn't show this file as being moved to res/linapple.conf, and it seems that it is not in fact deleted; it is still there in the repo, with size 0. I think it should probably be OK to just delete it entirely?
|
Have now tested it. Unfortunately, it does not work for my use case. It apparently still looks for files in the current directory: and with no font file, it is not usable, alas. |
|
Note, don't take #7 (comment) as a blocker, only as an indication that once this is goes in, I'll want to address that, somehow. SDL_GetBasePath sounds like it might work nicely, if A wrapper shell script might also suffice; I haven't really explored this possibility fully. I'll think about it. |
|
Strangely, even if I follow the directions in the readme for running it without installing, i.e. I get the same error as in #7 (comment), i.e. it looks like it's looking in the current directory for those bmps and fonts, but it says it can't find them. (Yet Will investigate, I suppose. I may open a PR against https://github.com/iKarith/linapple if I manage to fix anything, and/or to address the small things I saw in my code review, above. |
Small fixes against Greg's changes
|
That's all right—the working directory should be irrelevant to where it's run from. I know that's not generally the case on Windows, but it's what we want for Linux. LinApple needs to know where its files are and to load them from there regardless of where you run it from. Thanks for the fixes! I'll hold out merging this so @ghedger has a chance to eyeball it. I don't think even then I'll be ready to issue a merge into your trees just yet—Greg went through a fair amount of trouble to make the style consistent and … it isn't now. I'd also like to clean up the Debian package, add an EditorConfig file, etc. |
|
Yes, the install packaging definitely needs work - support more than Debian, install to a more sensible spot than /etc. It was just a first-pass, and I defer to my betters on packaging best practices. Thank you all who have contributed to this effort! |
|
Ah, I see what's going on now - it actually changes the current working directory to the user's home directory when it starts.
Well... for the record, having it always change directory to But, as I mentioned previously, my philosophy is not everyone's, so I won't push for this. If I can't figure out an elegant way to support it alongside this, I can just continue to maintain my fork. |
|
Greg, no worries about first passes for things not yet understood fully. You and Chris know far more about this codebase than I do, since I know only what little I've gleaned these past weeks. Chris has caught most of the rough edges I knew there'd be already. |
|
Chris, if you see anything else bugfixy or cleanupy in the next couple of days, feel free to bugfix or cleanup as needed in this repo. I'm going to merge and then spend some time seeing if I can't move us to SDL2 using SDLCL. I'll document what I'm doing in #5. |
|
Nope, no opposition here. Eventually I wanted to move to CMake actually, but autoconf would be fine.On May 23, 2018 11:57 AM, Joseph Carter <notifications@github.com> wrote:@iKarith commented on this pull request.
In Makefile:
+clean:
+ @$(RM) -rf $(BUILDDIR)
+ @$(RM) -rf $(TARGETDIR)
+
+#Full Clean, Objects and Binaries
+cleaner: clean
+ @$(RM) -rf $(TARGETDIR)
+ @$(RM) $(TARGET)-$(VERSION).deb
+
+#Pull in dependency info for *existing* .o files
+-include $(OBJECTS:.$(OBJEXT)=.$(DEPEXT))
+
+#Link
+
+$(TARGET): $(OBJECTS)
+ $(CC) $(LFLAGS) -o $(TARGETDIR)/$(TARGET) $^ $(LIB)
Are you opposed to autoconf or even automake? Some people are fanatically opposed because of how ridiculously overcomplicated it is, particularly in terms of what it generates, in the name of compatibility with 60 year old UNIX systems nobody's using anymore. I tried to love CMake, and I certainly love how it can generate all the project files for all the platforms, but it always felt like I was wrestling with it. Maybe I'm just more familiar with autoconf and potentially automake.
Which isn't to say that you don't wrestle with those too—I'm just probably quite able to write a configure.ac in an afternoon despite the fact that I've not done it for ten years.
LinApple's build isn't so complex that it really requires it, but if we want things to behave properly UNIXy, it doesn't get any more (pathologically) UNIX than autoconf at least. ;)
—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.
|
This may undo some of @cpressey's work (I hope not). If it doesn't, it should also go a long way toward @dabonetn's goals as well. That said, things are gonna be a bit rough with this as merged: