-
Notifications
You must be signed in to change notification settings - Fork 115
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
New mapper code #612
New mapper code #612
Conversation
- Mapper code rewrite, fixes and updates, additions - Expansion Audio updates - update VRC7 sound code (emu2413) - replace Sunsoft 5B sound code with emu2149 for better accuracy (including envelop and noise emulation) - Update FDS sound expansion (fds audio code from Mednafen with modification for fceumm' use) - Update NSF support - Add NSFE support - others (maybe)
Require for some mappers aka Game Doctor, etc
…trols for expansion audio
Adding it as-is
oh wow, big PR! This will take a hot minute to review intensively, so is there anything we should be on the look out for in a quicker skim? |
code-wise, not sure. maybe a heads up especially for those relying on savestates since this will be broken in this PR (and the succeding PRs, probably) i DO need to know if there is a better way to handle core options for translations. its kinda tedious to add/remove/change options since you have to deal with all languages one by one to update. maybe, a separate macro/inline/define file for each language used, then a common struct which labels will be replaced depending on language. right now, you have to make/update struct on each. |
Well, translations are done on the CrowdIn platform, whenever ppl submit new translations there, the repo pulls in new code. Also I gave you write permissions for this repo too. Not sure if the invite expired, it was sent sometime ago. |
Looks alright to me and first testing seems OK. I'll be asking dusanfamicom for some more thorough testing. He can file new issue reports based on his findings. |
Yes, this is what I was thinking, as well, but wasn't sure. In that case, you don't need to worry about anything but the english language and then the others are handled by volunteers via crowdin. |
did it break crowdin fetch thingie though? |
I don't think it should but we'll find out soon enough if/when ppl complain on Crowdin. |
it said expired. |
Hi, I got some feedback from dusan.
I asked several times if he could be more specific but that is all what he came back with. He says these mappers did work before this PR. Is it perhaps possible you can leave out the refactors, maybe revert the PR and just pull in the other changes/additions instead that won't cause possible regressions to existing mappers? Basically we might have to be more careful with the mapper refactors to avoid breakage of existing mappers. |
nothing can be fixed IF no details are provided. if it worked before, then its just a minor fix in the refactor, if its even a code error and not human error. since this was practically "refactored/converted" from my own repo which was already tested (at least for the roms file i have at hand).. the modifications are too many to do this all over again one commit at a time... you can just revert this repo to what it was and make it a static core, make mine a new repo if you want. but i wont be porting the modifications again one by one. the mapper code alone is done by a set and aside from it a big work to do, its incomplete as far as the implementations i was suppose to that needed the refactoring of the entire mapper code to begin with. |
@LibretroAdmin can they at least give us a list of games they tried that didn't work? |
I agree that more info should be provided. I'll try my best again. I don't really want any breakdown in relationship over this so I'll try my best to see if he can get us something concrete at least that we can work with to improve things. |
just revert it so people can still use the old core if they want that one. make a separate repo for new fceumm with modifications i can work with or something. i was already woking on new sets to push and this news just make me think its not worth it if the people testing wont even bother giving details. and i dont even trust those CN guys, if theyre the only ones making this isses. |
Let's not give up motivation yet. I'll keep prodding along to see if I can get some useful information out of him. Are there any regression tests for mappers so we can determine if changes break something? |
yeah, more waiting.... |
i have just tested a bunch of multicart based roms, including the newer released ones just a few months ago and everything works, as far as loading the rom itself and selecting a few games. we should get proper description of the issues instead of making "REVERT THE PR" the defacto when there are issue instead of fixing and moving forward especially when someone is availble to work whichever the kinks are. if its "working before", no reason it will not work, just some kinks to fix due to the conversion from one already about 60% complete repo to here which practially have to compensate for the changes. if you want this repo to be static, then just revert it back and freeze it for maintenance reason. just be sure whan you want. its always been like this, the waiting causing momentum to break coz we have to listen to reports with missing details. |
@LibretroAdmin @hunterk @hizzlekizzle Seriously, you need to revert these changes if im not able to continue to work on this repo. This PR is suppose to be a part of a continues PR i was hoping to be able to do to make it a bit more complete and in sync with my current fork of fceumm. it was not intended to be just 1 PR, even if it works in some places. Since my PR are getting questioned, not even so much of details then i cant continue working ok it. so much for that "i trust your PR" talk eh. |
Alright @negativeExponent , sorry about all that. I talked to NRS and basically I've come to the conclusion that you can continue with the followup PRs. We will see where this goes. Sorry for the whole detour. So basically proceed as if this entire convo didn't happen. If someone can actually give me solid feedback on whatever broke then I can present it to you. Until then it won't prevent development from continuing. |
so your not taking sides and yet the solution is to revert instead of really taking the proper course, and that is to get the info of the problems to resolve. again, if its working before, there is no reason it wont work anymore. since these are the CN guys, i think its more of the "custom hacked" mappers that are specifically modified for specific hacks. i only add roms/mappers i can test with. if someone wants something not there, its can easiliy be added. but i heard enough. this was a waste of time to be honest. again so much for "i trust your PR" might want to take my rights as well on PX68K as well. |
What are you talking about? I said just now that nothing needs to be reverted and that you can send any followup PRs you had prepared already. I was referring to this
So I thought you had PRs in waiting. That's what i meant. What you concluded is the exact opposite of what I just said. So no, what I'm saying is I side with you. Hell, you can even commit it directly if you feel like the PR is taking too long to merge. So again, nothing needs to be reverted.
No, that is EXACTLY what I decided upon. That's why I said nothing needs to be reverted and you can send the next PRs you had lying in wait. I said before (re-read the last comment) that if there are issues with mappers he needs to file a proper bug report on that mapper and report back before we take action. That's what I said. So again, there is no problem here. I already agreed to go with your approach. |
Sent another invite since the previous one had expired. |
lots of changes. see if there is anything worth changing first before we process ig a big merge. Atm, this is the jist of the entire mapper rework, modified to work with this codebase. Regressions might be in place, maybe some incorrect conversions. better alot can test other than just me. The other modifications can be added later bits by bits.
See 1st commit for majority of the change.
BTW, with regards to core options for language translations... is there a way to make it easier to add or remove options without having to do it over and over againt for every language that was added?
@LibretroAdmin @hunterk @hizzlekizzle for review
btw, some unused code left behind, will be cleaned up later when things work or is stable enough.